Merge pull request #250 from perillo/improve-test-coverage-2

Improve test coverage

build.zig

@@ -18,7 +18,8 @@ pub const Exercise = struct {
     main_file: []const u8,
 
     /// This is the desired output of the program.
-    /// A program passes if its output ends with this string.
+    /// A program passes if its output, excluding trailing whitespace, is equal
+    /// to this string.
     output: []const u8,
 
     /// This is an optional hint to give if the program does not succeed.
@@ -365,7 +366,7 @@ const exercises = [_]Exercise{
     },
     .{
         .main_file = "068_comptime3.zig",
-        .output = "Minnow (1:32, 4 x 2)\nShark (1:16, 8 x 5)\nWhale (1:1, 143 x 95)\n",
+        .output = "Minnow (1:32, 4 x 2)\nShark (1:16, 8 x 5)\nWhale (1:1, 143 x 95)",
     },
     .{
         .main_file = "069_comptime4.zig",
@@ -516,7 +517,7 @@ const exercises = [_]Exercise{
     },
     .{
         .main_file = "099_formatting.zig",
-        .output = "\n X |  1   2   3   4   5   6   7   8   9  10  11  12  13  14  15 \n---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+\n 1 |  1   2   3   4   5   6   7   8   9  10  11  12  13  14  15 \n\n 2 |  2   4   6   8  10  12  14  16  18  20  22  24  26  28  30 \n\n 3 |  3   6   9  12  15  18  21  24  27  30  33  36  39  42  45 \n\n 4 |  4   8  12  16  20  24  28  32  36  40  44  48  52  56  60 \n\n 5 |  5  10  15  20  25  30  35  40  45  50  55  60  65  70  75 \n\n 6 |  6  12  18  24  30  36  42  48  54  60  66  72  78  84  90 \n\n 7 |  7  14  21  28  35  42  49  56  63  70  77  84  91  98 105 \n\n 8 |  8  16  24  32  40  48  56  64  72  80  88  96 104 112 120 \n\n 9 |  9  18  27  36  45  54  63  72  81  90  99 108 117 126 135 \n\n10 | 10  20  30  40  50  60  70  80  90 100 110 120 130 140 150 \n\n11 | 11  22  33  44  55  66  77  88  99 110 121 132 143 154 165 \n\n12 | 12  24  36  48  60  72  84  96 108 120 132 144 156 168 180 \n\n13 | 13  26  39  52  65  78  91 104 117 130 143 156 169 182 195 \n\n14 | 14  28  42  56  70  84  98 112 126 140 154 168 182 196 210 \n\n15 | 15  30  45  60  75  90 105 120 135 150 165 180 195 210 225 \n\n",
+        .output = "\n X |  1   2   3   4   5   6   7   8   9  10  11  12  13  14  15 \n---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+\n 1 |  1   2   3   4   5   6   7   8   9  10  11  12  13  14  15 \n\n 2 |  2   4   6   8  10  12  14  16  18  20  22  24  26  28  30 \n\n 3 |  3   6   9  12  15  18  21  24  27  30  33  36  39  42  45 \n\n 4 |  4   8  12  16  20  24  28  32  36  40  44  48  52  56  60 \n\n 5 |  5  10  15  20  25  30  35  40  45  50  55  60  65  70  75 \n\n 6 |  6  12  18  24  30  36  42  48  54  60  66  72  78  84  90 \n\n 7 |  7  14  21  28  35  42  49  56  63  70  77  84  91  98 105 \n\n 8 |  8  16  24  32  40  48  56  64  72  80  88  96 104 112 120 \n\n 9 |  9  18  27  36  45  54  63  72  81  90  99 108 117 126 135 \n\n10 | 10  20  30  40  50  60  70  80  90 100 110 120 130 140 150 \n\n11 | 11  22  33  44  55  66  77  88  99 110 121 132 143 154 165 \n\n12 | 12  24  36  48  60  72  84  96 108 120 132 144 156 168 180 \n\n13 | 13  26  39  52  65  78  91 104 117 130 143 156 169 182 195 \n\n14 | 14  28  42  56  70  84  98 112 126 140 154 168 182 196 210 \n\n15 | 15  30  45  60  75  90 105 120 135 150 165 180 195 210 225",
     },
     .{
         .main_file = "999_the_end.zig",
@@ -758,20 +759,20 @@ const ZiglingStep = struct {
             return err;
         };
 
-        // Allow up to 1 MB of stdout capture
+        // Allow up to 1 MB of stdout capture.
         const max_output_len = 1 * 1024 * 1024;
         const output = if (self.exercise.check_stdout)
             try child.stdout.?.reader().readAllAlloc(self.builder.allocator, max_output_len)
         else
             try child.stderr.?.reader().readAllAlloc(self.builder.allocator, max_output_len);
 
-        // at this point stdout is closed, wait for the process to terminate
+        // At this point stdout is closed, wait for the process to terminate.
         const term = child.wait() catch |err| {
             print("{s}Unable to spawn {s}: {s}{s}\n", .{ red_text, argv[0], @errorName(err), reset_text });
             return err;
         };
 
-        // make sure it exited cleanly.
+        // Make sure it exited cleanly.
         switch (term) {
             .Exited => |code| {
                 if (code != 0) {
@@ -785,10 +786,10 @@ const ZiglingStep = struct {
             },
         }
 
-        // validate the output
+        // Validate the output.
         const trimOutput = std.mem.trimRight(u8, output, " \r\n");
         const trimExerciseOutput = std.mem.trimRight(u8, self.exercise.output, " \r\n");
-        if (std.mem.indexOf(u8, trimOutput, trimExerciseOutput) == null or trimOutput.len != trimExerciseOutput.len) {
+        if (!std.mem.eql(u8, trimOutput, trimExerciseOutput)) {
             print(
                 \\
                 \\{s}----------- Expected this output -----------{s}
@@ -801,7 +802,7 @@ const ZiglingStep = struct {
             return error.InvalidOutput;
         }
 
-        print("{s}PASSED:\n{s}{s}\n", .{ green_text, output, reset_text });
+        print("{s}PASSED:\n{s}{s}\n\n", .{ green_text, trimOutput, reset_text });
     }
 
     // The normal compile step calls os.exit, so we can't use it as a library :(
@@ -1057,18 +1058,31 @@ const SkipStep = struct {
     }
 };
 
-// Check that each exercise number, excluding the last, forms the sequence `[1, exercise.len)`.
+// Check that each exercise number, excluding the last, forms the sequence
+// `[1, exercise.len)`.
+//
+// Additionally check that the output field does not contain trailing whitespace.
 fn validate_exercises() bool {
-    // Don't use the "multi-object for loop" syntax, in order to avoid a syntax error with old Zig
+    // Don't use the "multi-object for loop" syntax, in order to avoid a syntax
-    // compilers.
+    // error with old Zig compilers.
     var i: usize = 0;
     for (exercises[0 .. exercises.len - 1]) |ex| {
         i += 1;
         if (ex.number() != i) {
-            print(
+            print("exercise {s} has an incorrect number: expected {}, got {s}\n", .{
-                "exercise {s} has an incorrect number: expected {}, got {s}\n",
+                ex.main_file,
-                .{ ex.main_file, i, ex.key() },
+                i,
-            );
+                ex.key(),
+            });
+
+            return false;
+        }
+
+        const output = std.mem.trimRight(u8, ex.output, " \r\n");
+        if (output.len != ex.output.len) {
+            print("exercise {s} output field has extra trailing whitespace\n", .{
+                ex.main_file,
+            });
 
             return false;
         }

test/tests.zig

@@ -2,11 +2,16 @@ const std = @import("std");
 const root = @import("../build.zig");
 
 const debug = std.debug;
+const fmt = std.fmt;
 const fs = std.fs;
+const mem = std.mem;
 
+const Allocator = std.mem.Allocator;
 const Build = std.build;
-const Step = Build.Step;
+const FileSource = std.Build.FileSource;
+const Reader = fs.File.Reader;
 const RunStep = std.Build.RunStep;
+const Step = Build.Step;
 
 const Exercise = root.Exercise;
 
@@ -18,30 +23,27 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
     const outdir = "patches/healed";
 
     fs.cwd().makePath(outdir) catch |err| {
-        debug.print("unable to make '{s}': {s}\n", .{ outdir, @errorName(err) });
+        return fail(step, "unable to make '{s}': {s}\n", .{ outdir, @errorName(err) });
-
+    };
-        return step;
+    heal(b.allocator, exercises, outdir) catch |err| {
+        return fail(step, "unable to heal exercises: {s}\n", .{@errorName(err)});
     };
 
     {
+        // Test that `zig build -Dhealed -Dn=n test` selects the nth exercise.
         const case_step = createCase(b, "case-1");
 
-        // Test that `zig build -Dn=n -Dhealed test` selects the nth exercise.
         var i: usize = 0;
         for (exercises[0 .. exercises.len - 1]) |ex| {
             i += 1;
             if (ex.skip) continue;
 
-            const patch = PatchStep.create(b, ex, outdir);
-
             const cmd = b.addSystemCommand(
-                &.{ b.zig_exe, "build", b.fmt("-Dn={}", .{i}), "-Dhealed", "test" },
+                &.{ b.zig_exe, "build", "-Dhealed", b.fmt("-Dn={}", .{i}), "test" },
             );
-            cmd.setName(b.fmt("zig build -D={} -Dhealed test", .{i}));
+            cmd.setName(b.fmt("zig build -Dhealed -Dn={} test", .{i}));
             cmd.expectExitCode(0);
-            cmd.step.dependOn(&patch.step);
 
-            // Some exercise output has an extra space character.
             if (ex.check_stdout)
                 expectStdOutMatch(cmd, ex.output)
             else
@@ -54,18 +56,18 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
     }
 
     {
+        // Test that `zig build -Dhealed -Dn=n test` skips disabled esercises.
         const case_step = createCase(b, "case-2");
 
-        // Test that `zig build -Dn=n -Dhealed test` skips disabled esercises.
         var i: usize = 0;
         for (exercises[0 .. exercises.len - 1]) |ex| {
             i += 1;
             if (!ex.skip) continue;
 
             const cmd = b.addSystemCommand(
-                &.{ b.zig_exe, "build", b.fmt("-Dn={}", .{i}), "-Dhealed", "test" },
+                &.{ b.zig_exe, "build", "-Dhealed", b.fmt("-Dn={}", .{i}), "test" },
             );
-            cmd.setName(b.fmt("zig build -D={} -Dhealed test", .{i}));
+            cmd.setName(b.fmt("zig build -Dhealed -Dn={} test", .{i}));
             cmd.expectExitCode(0);
             cmd.expectStdOutEqual("");
             expectStdErrMatch(cmd, b.fmt("{s} skipped", .{ex.main_file}));
@@ -76,8 +78,63 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step {
         step.dependOn(case_step);
     }
 
-    const cleanup = b.addRemoveDirTree(outdir);
+    {
-    step.dependOn(&cleanup.step);
+        // Test that `zig build -Dhealed` process all the exercises in order.
+        const case_step = createCase(b, "case-3");
+
+        // TODO: when an exercise is modified, the cache is not invalidated.
+        const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dhealed" });
+        cmd.setName("zig build -Dhealed");
+        cmd.expectExitCode(0);
+
+        const stderr = cmd.captureStdErr();
+        const verify = CheckStep.create(b, exercises, stderr, true);
+        verify.step.dependOn(&cmd.step);
+
+        case_step.dependOn(&verify.step);
+
+        step.dependOn(case_step);
+    }
+
+    {
+        // Test that `zig build -Dhealed -Dn=1 start` process all the exercises
+        // in order.
+        const case_step = createCase(b, "case-4");
+
+        // TODO: when an exercise is modified, the cache is not invalidated.
+        const cmd = b.addSystemCommand(
+            &.{ b.zig_exe, "build", "-Dhealed", "-Dn=1", "start" },
+        );
+        cmd.setName("zig build -Dhealed -Dn=1 start");
+        cmd.expectExitCode(0);
+
+        const stderr = cmd.captureStdErr();
+        const verify = CheckStep.create(b, exercises, stderr, false);
+        verify.step.dependOn(&cmd.step);
+
+        case_step.dependOn(&verify.step);
+
+        step.dependOn(case_step);
+    }
+
+    {
+        // Test that `zig build -Dn=1` prints the hint.
+        const case_step = createCase(b, "case-5");
+
+        const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dn=1" });
+        cmd.setName("zig build -Dn=1");
+        cmd.expectExitCode(1);
+        expectStdErrMatch(cmd, exercises[0].hint);
+
+        case_step.dependOn(&cmd.step);
+
+        step.dependOn(case_step);
+    }
+
+    // Don't add the cleanup step, since it may delete outdir while a test case
+    // is running.
+    //const cleanup = b.addRemoveDirTree(outdir);
+    //step.dependOn(&cleanup.step);
 
     return step;
 }
@@ -93,28 +150,30 @@ fn createCase(b: *Build, name: []const u8) *Step {
     return case_step;
 }
 
-// Apply a patch to the specified exercise.
+// Check the output of `zig build` or `zig build -Dn=1 start`.
-const PatchStep = struct {
+const CheckStep = struct {
-    const join = fs.path.join;
-
-    const exercises_path = "exercises";
-    const patches_path = "patches/patches";
-
     step: Step,
-    exercise: Exercise,
+    exercises: []const Exercise,
-    outdir: []const u8,
+    stderr: FileSource,
+    has_logo: bool,
 
-    pub fn create(owner: *Build, exercise: Exercise, outdir: []const u8) *PatchStep {
+    pub fn create(
-        const self = owner.allocator.create(PatchStep) catch @panic("OOM");
+        owner: *Build,
+        exercises: []const Exercise,
+        stderr: FileSource,
+        has_logo: bool,
+    ) *CheckStep {
+        const self = owner.allocator.create(CheckStep) catch @panic("OOM");
         self.* = .{
             .step = Step.init(.{
                 .id = .custom,
-                .name = owner.fmt("patch {s}", .{exercise.main_file}),
+                .name = "check",
                 .owner = owner,
                 .makeFn = make,
             }),
-            .exercise = exercise,
+            .exercises = exercises,
-            .outdir = outdir,
+            .stderr = stderr,
+            .has_logo = has_logo,
         };
 
         return self;
@@ -122,25 +181,165 @@ const PatchStep = struct {
 
     fn make(step: *Step, _: *std.Progress.Node) !void {
         const b = step.owner;
-        const self = @fieldParentPtr(PatchStep, "step", step);
+        const self = @fieldParentPtr(CheckStep, "step", step);
-        const exercise = self.exercise;
+        const exercises = self.exercises;
-        const name = exercise.baseName();
+
+        const stderr_file = try fs.cwd().openFile(
+            self.stderr.getPath(b),
+            .{ .mode = .read_only },
+        );
+        defer stderr_file.close();
+
+        const stderr = stderr_file.reader();
+        for (exercises) |ex| {
+            if (ex.number() == 1 and self.has_logo) {
+                // Skip the logo.
+                var buf: [80]u8 = undefined;
+
+                var lineno: usize = 0;
+                while (lineno < 8) : (lineno += 1) {
+                    _ = try readLine(stderr, &buf);
+                }
+            }
+            try check_output(step, ex, stderr);
+        }
+    }
+
+    fn check_output(step: *Step, exercise: Exercise, reader: Reader) !void {
+        const b = step.owner;
+
+        var buf: [1024]u8 = undefined;
+        if (exercise.skip) {
+            {
+                const actual = try readLine(reader, &buf) orelse "EOF";
+                const expect = b.fmt("Skipping {s}", .{exercise.main_file});
+                try check(step, exercise, expect, actual);
+            }
+
+            {
+                const actual = try readLine(reader, &buf) orelse "EOF";
+                try check(step, exercise, "", actual);
+            }
+
+            return;
+        }
+
+        {
+            const actual = try readLine(reader, &buf) orelse "EOF";
+            const expect = b.fmt("Compiling {s}...", .{exercise.main_file});
+            try check(step, exercise, expect, actual);
+        }
+
+        {
+            const actual = try readLine(reader, &buf) orelse "EOF";
+            const expect = b.fmt("Checking {s}...", .{exercise.main_file});
+            try check(step, exercise, expect, actual);
+        }
+
+        {
+            const actual = try readLine(reader, &buf) orelse "EOF";
+            const expect = "PASSED:";
+            try check(step, exercise, expect, actual);
+        }
+
+        // Skip the exercise output.
+        const nlines = 1 + mem.count(u8, exercise.output, "\n") + 1;
+        var lineno: usize = 0;
+        while (lineno < nlines) : (lineno += 1) {
+            _ = try readLine(reader, &buf) orelse @panic("EOF");
+        }
+    }
+
+    fn check(
+        step: *Step,
+        exercise: Exercise,
+        expect: []const u8,
+        actual: []const u8,
+    ) !void {
+        if (!mem.eql(u8, expect, actual)) {
+            return step.fail("{s}: expected to see \"{s}\", found \"{s}\"", .{
+                exercise.main_file,
+                expect,
+                actual,
+            });
+        }
+    }
+
+    fn readLine(reader: fs.File.Reader, buf: []u8) !?[]const u8 {
+        if (try reader.readUntilDelimiterOrEof(buf, '\n')) |line| {
+            return mem.trimRight(u8, line, " \r\n");
+        }
+
+        return null;
+    }
+};
+
+// A step that will fail.
+const FailStep = struct {
+    step: Step,
+    error_msg: []const u8,
+
+    pub fn create(owner: *Build, error_msg: []const u8) *FailStep {
+        const self = owner.allocator.create(FailStep) catch @panic("OOM");
+        self.* = .{
+            .step = Step.init(.{
+                .id = .custom,
+                .name = "fail",
+                .owner = owner,
+                .makeFn = make,
+            }),
+            .error_msg = error_msg,
+        };
+
+        return self;
+    }
+
+    fn make(step: *Step, _: *std.Progress.Node) !void {
+        const b = step.owner;
+        const self = @fieldParentPtr(FailStep, "step", step);
+
+        try step.result_error_msgs.append(b.allocator, self.error_msg);
+        return error.MakeFailed;
+    }
+};
+
+// A variant of `std.Build.Step.fail` that does not return an error so that it
+// can be used in the configuration phase.  It returns a FailStep, so that the
+// error will be cleanly handled by the build runner.
+fn fail(step: *Step, comptime format: []const u8, args: anytype) *Step {
+    const b = step.owner;
+
+    const fail_step = FailStep.create(b, b.fmt(format, args));
+    step.dependOn(&fail_step.step);
+
+    return step;
+}
+
+// Heals all the exercises.
+fn heal(allocator: Allocator, exercises: []const Exercise, outdir: []const u8) !void {
+    const join = fs.path.join;
+
+    const exercises_path = "exercises";
+    const patches_path = "patches/patches";
+
+    for (exercises) |ex| {
+        const name = ex.baseName();
 
         // Use the POSIX patch variant.
-        const file = join(b.allocator, &.{ exercises_path, exercise.main_file }) catch
+        const file = try join(allocator, &.{ exercises_path, ex.main_file });
-            @panic("OOM");
+        const patch = b: {
-        const patch = join(b.allocator, &.{ patches_path, b.fmt("{s}.patch", .{name}) }) catch
+            const patch_name = try fmt.allocPrint(allocator, "{s}.patch", .{name});
-            @panic("OOM");
+            break :b try join(allocator, &.{ patches_path, patch_name });
-        const output = join(b.allocator, &.{ self.outdir, exercise.main_file }) catch
+        };
-            @panic("OOM");
+        const output = try join(allocator, &.{ outdir, ex.main_file });
 
         const argv = &.{ "patch", "-i", patch, "-o", output, file };
 
-        var child = std.process.Child.init(argv, b.allocator);
+        var child = std.process.Child.init(argv, allocator);
         child.stdout_behavior = .Ignore; // the POSIX standard says that stdout is not used
         _ = try child.spawnAndWait();
     }
-};
+}
 
 //
 // Missing functions from std.Build.RunStep