From 74b48192e4ab3f9b8dbc5ac1e40f295f88a976d8 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 2 May 2023 10:51:59 +0200 Subject: [PATCH 1/2] test: don't run heal during configuration phase In order to simplify the code, the heal function is called during the configuration phase, thus resulting in the function being always called when the build.zig file is run. This behavior unfortunately causes a serious issue when the user fix a broken exercise and, during the next step, the heal function tries to heal the fixed exercise resulting in GNU patch assuming an attempt to reverse a patch, waiting for input from the terminal. Run the heal function from the new HealStep step, so that it is called only during tests. Rename the outdir constant to work_path, for consistency with build.zig. Fixes #272 --- test/tests.zig | 59 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/test/tests.zig b/test/tests.zig index 8786d91..752ca50 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -20,15 +20,14 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { // We should use a temporary path, but it will make the implementation of // `build.zig` more complex. - const outdir = "patches/healed"; + const work_path = "patches/healed"; - fs.cwd().makePath(outdir) catch |err| { - return fail(step, "unable to make '{s}': {s}\n", .{ outdir, @errorName(err) }); - }; - heal(b.allocator, exercises, outdir) catch |err| { - return fail(step, "unable to heal exercises: {s}\n", .{@errorName(err)}); + fs.cwd().makePath(work_path) catch |err| { + return fail(step, "unable to make '{s}': {s}\n", .{ work_path, @errorName(err) }); }; + const heal_step = HealStep.create(b, exercises, work_path); + { // Test that `zig build -Dhealed -Dn=n test` selects the nth exercise. const case_step = createCase(b, "case-1"); @@ -49,6 +48,8 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { else expectStdErrMatch(cmd, ex.output); + cmd.step.dependOn(&heal_step.step); + case_step.dependOn(&cmd.step); } @@ -72,6 +73,8 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { cmd.expectStdOutEqual(""); expectStdErrMatch(cmd, b.fmt("{s} skipped", .{ex.main_file})); + cmd.step.dependOn(&heal_step.step); + case_step.dependOn(&cmd.step); } @@ -86,6 +89,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { const cmd = b.addSystemCommand(&.{ b.zig_exe, "build", "-Dhealed" }); cmd.setName("zig build -Dhealed"); cmd.expectExitCode(0); + cmd.step.dependOn(&heal_step.step); const stderr = cmd.captureStdErr(); const verify = CheckStep.create(b, exercises, stderr, true); @@ -107,6 +111,7 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { ); cmd.setName("zig build -Dhealed -Dn=1 start"); cmd.expectExitCode(0); + cmd.step.dependOn(&heal_step.step); const stderr = cmd.captureStdErr(); const verify = CheckStep.create(b, exercises, stderr, false); @@ -126,14 +131,16 @@ pub fn addCliTests(b: *std.Build, exercises: []const Exercise) *Step { cmd.expectExitCode(1); expectStdErrMatch(cmd, exercises[0].hint); + cmd.step.dependOn(&heal_step.step); + 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); + // Don't add the cleanup step, since it may delete work_path while a test + // case is running. + //const cleanup = b.addRemoveDirTree(work_path); //step.dependOn(&cleanup.step); return step; @@ -315,8 +322,38 @@ fn fail(step: *Step, comptime format: []const u8, args: anytype) *Step { return step; } +// A step that heals exercises. +const HealStep = struct { + step: Step, + exercises: []const Exercise, + work_path: []const u8, + + pub fn create(owner: *Build, exercises: []const Exercise, work_path: []const u8) *HealStep { + const self = owner.allocator.create(HealStep) catch @panic("OOM"); + self.* = .{ + .step = Step.init(.{ + .id = .custom, + .name = "heal", + .owner = owner, + .makeFn = make, + }), + .exercises = exercises, + .work_path = work_path, + }; + + return self; + } + + fn make(step: *Step, _: *std.Progress.Node) !void { + const b = step.owner; + const self = @fieldParentPtr(HealStep, "step", step); + + return heal(b.allocator, self.exercises, self.work_path); + } +}; + // Heals all the exercises. -fn heal(allocator: Allocator, exercises: []const Exercise, outdir: []const u8) !void { +fn heal(allocator: Allocator, exercises: []const Exercise, work_path: []const u8) !void { const join = fs.path.join; const exercises_path = "exercises"; @@ -331,7 +368,7 @@ fn heal(allocator: Allocator, exercises: []const Exercise, outdir: []const u8) ! const patch_name = try fmt.allocPrint(allocator, "{s}.patch", .{name}); break :b try join(allocator, &.{ patches_path, patch_name }); }; - const output = try join(allocator, &.{ outdir, ex.main_file }); + const output = try join(allocator, &.{ work_path, ex.main_file }); const argv = &.{ "patch", "-i", patch, "-o", output, "-s", file }; From 7a40c4584e9118e1b218ff7c424e524b046abfbc Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Tue, 2 May 2023 11:06:58 +0200 Subject: [PATCH 2/2] Restore unit tests --- .github/workflows/ci.yml | 36 ++++++++++++++++++------------------ build.zig | 5 ++--- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab821c6..d071d8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,21 +27,21 @@ jobs: - name: Check compatibility with old Zig compilers run: ci/compat.sh -# test: -# name: Unit Tests -# strategy: -# matrix: -# os: [ubuntu-latest, windows-latest, macos-latest] -# runs-on: ${{ matrix.os }} -# timeout-minutes: 30 -# steps: -# - name: Checkout -# uses: actions/checkout@v3 -# -# - name: Setup Zig -# uses: goto-bus-stop/setup-zig@v2 -# with: -# version: master -# -# - name: Run unit tests -# run: zig build test + test: + name: Unit Tests + strategy: + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Setup Zig + uses: goto-bus-stop/setup-zig@v2 + with: + version: master + + - name: Run unit tests + run: zig build test diff --git a/build.zig b/build.zig index 38890c2..383a231 100644 --- a/build.zig +++ b/build.zig @@ -210,9 +210,8 @@ pub fn build(b: *Build) !void { } ziglings_step.dependOn(prev_step); - // Disabled, see issue 272 - // const test_step = b.step("test", "Run all the tests"); - // // test_step.dependOn(tests.addCliTests(b, &exercises)); + const test_step = b.step("test", "Run all the tests"); + test_step.dependOn(tests.addCliTests(b, &exercises)); } var use_color_escapes = false;