authorgravatar for topolarity@tapscott.meCody Tapscott <topolarity@tapscott.me> 2022-04-11 14:52:09-07:00
committergravatar for topolarity@tapscott.meCody Tapscott <topolarity@tapscott.me> 2022-04-11 15:35:28-07:00
log879b5627799b82ba7bcb4f15a8f0f517e014b222
tree6ec93d94eb1b5d05cb920ac40523d123edc3d046
parent7851377e95f1a0e9467a7e69dd506b47a4f45b87

Add file support for incremental error tests

Compile error test cases can now be given as a sequence of files: - "foo.1.zig" - "foo.2.zig" - "foo.3.zig" - etc. This sequence of files is tested as incremental compilation updates to a single "foo.zig" source file. To help avoid mistakes, we enforce strict ordering for these files. "foo.zig" cannot co-exist with "foo.X.zig", the sequence must include "foo.1.zig", and no numbers may be skipped.

1 files changed, 113 insertions(+), 7 deletions(-)

src/test.zig+113-7
...@@ -46,6 +46,7 @@ test {...@@ -46,6 +46,7 @@ test {
46 defer stage2_dir.close();46 defer stage2_dir.close();
4747
48 // TODO make this incremental once the bug is solved that it triggers48 // TODO make this incremental once the bug is solved that it triggers
49 // See: https://github.com/ziglang/zig/issues/11344
49 ctx.addErrorCasesFromDir("stage2", stage2_dir, .stage2, .Obj, false, .independent);50 ctx.addErrorCasesFromDir("stage2", stage2_dir, .stage2, .Obj, false, .independent);
50 }51 }
5152
...@@ -653,7 +654,14 @@ pub const TestContext = struct {...@@ -653,7 +654,14 @@ pub const TestContext = struct {
653 case.compiles(fixed_src);654 case.compiles(fixed_src);
654 }655 }
655656
656 const Strategy = enum { incremental, independent };657 const Strategy = enum {
658 /// Execute tests as independent compilations, unless they are explicitly
659 /// incremental ("foo.1.zig", "foo.2.zig", etc.)
660 independent,
661 /// Execute all tests as incremental updates to a single compilation. Explicitly
662 /// incremental tests ("foo.1.zig", "foo.2.zig", etc.) still execute in order
663 incremental,
664 };
657665
658 /// Adds a compile-error test for each file in the provided directory, using the666 /// Adds a compile-error test for each file in the provided directory, using the
659 /// selected backend and output mode. If `one_test_case_per_file` is true, a new667 /// selected backend and output mode. If `one_test_case_per_file` is true, a new
...@@ -681,6 +689,66 @@ pub const TestContext = struct {...@@ -681,6 +689,66 @@ pub const TestContext = struct {
681 };689 };
682 }690 }
683691
692 /// For a filename in the format "<filename>.X.<ext>" or "<filename>.<ext>", returns
693 /// "<filename>", "<ext>" and X parsed as a decimal number. If X is not present, or
694 /// cannot be parsed as a decimal number, it is treated as part of <filename>
695 fn getTestFileNameParts(name: []const u8) struct {
696 base_name: []const u8,
697 file_ext: []const u8,
698 test_index: ?usize,
699 } {
700 const file_ext = std.fs.path.extension(name);
701 const trimmed = name[0 .. name.len - file_ext.len]; // Trim off ".<ext>"
702 const maybe_index = std.fs.path.extension(trimmed); // Extract ".X"
703
704 // Attempt to parse index
705 const index: ?usize = if (maybe_index.len > 0)
706 std.fmt.parseInt(usize, maybe_index[1..], 10) catch null
707 else
708 null;
709
710 // Adjust "<filename>" extent based on parsing success
711 const base_name_end = trimmed.len - if (index != null) maybe_index.len else 0;
712 return .{
713 .base_name = name[0..base_name_end],
714 .file_ext = if (file_ext.len > 0) file_ext[1..] else file_ext,
715 .test_index = index,
716 };
717 }
718
719 /// Sort test filenames in-place, so that incremental test cases ("foo.1.zig",
720 /// "foo.2.zig", etc.) are contiguous and appear in numerical order.
721 fn sortTestFilenames(
722 filenames: [][]const u8,
723 ) void {
724 const Context = struct {
725 pub fn lessThan(_: @This(), a: []const u8, b: []const u8) bool {
726 const a_parts = getTestFileNameParts(a);
727 const b_parts = getTestFileNameParts(b);
728
729 // Sort "<base_name>.X.<file_ext>" based on "<base_name>" and "<file_ext>" first
730 return switch (std.mem.order(u8, a_parts.base_name, b_parts.base_name)) {
731 .lt => true,
732 .gt => false,
733 .eq => switch (std.mem.order(u8, a_parts.file_ext, b_parts.file_ext)) {
734 .lt => true,
735 .gt => false,
736 .eq => b: { // a and b differ only in their ".X" part
737
738 // Sort "<base_name>.<file_ext>" before any "<base_name>.X.<file_ext>"
739 if (a_parts.test_index == null) break :b true;
740 if (b_parts.test_index == null) break :b false;
741
742 // Make sure that incremental tests appear in linear order
743 return a_parts.test_index.? < b_parts.test_index.?;
744 },
745 },
746 };
747 }
748 };
749 std.sort.sort([]const u8, filenames, Context{}, Context.lessThan);
750 }
751
684 fn addErrorCasesFromDirInner(752 fn addErrorCasesFromDirInner(
685 ctx: *TestContext,753 ctx: *TestContext,
686 name: []const u8,754 name: []const u8,
...@@ -696,6 +764,9 @@ pub const TestContext = struct {...@@ -696,6 +764,9 @@ pub const TestContext = struct {
696 var opt_case: ?*Case = null;764 var opt_case: ?*Case = null;
697765
698 var it = dir.iterate();766 var it = dir.iterate();
767 var filenames = std.ArrayList([]const u8).init(ctx.arena);
768 defer filenames.deinit();
769
699 while (try it.next()) |entry| {770 while (try it.next()) |entry| {
700 if (entry.kind != .File) continue;771 if (entry.kind != .File) continue;
701772
...@@ -704,11 +775,46 @@ pub const TestContext = struct {...@@ -704,11 +775,46 @@ pub const TestContext = struct {
704 .unknown => continue,775 .unknown => continue,
705 else => {},776 else => {},
706 }777 }
778 try filenames.append(try ctx.arena.dupe(u8, entry.name));
779 }
780
781 // Sort filenames, so that incremental tests are contiguous and in-order
782 sortTestFilenames(filenames.items);
783
784 var prev_filename: []const u8 = "";
785 for (filenames.items) |filename| {
786 current_file.* = filename;
787
788 { // First, check if this file is part of an incremental update sequence
789
790 // Split filename into "<base_name>.<index>.<file_ext>"
791 const prev_parts = getTestFileNameParts(prev_filename);
792 const new_parts = getTestFileNameParts(filename);
707793
708 current_file.* = try ctx.arena.dupe(u8, entry.name);794 // If base_name and file_ext match, these files are in the same test sequence
795 // and the new one should be the incremented version of the previous test
796 if (std.mem.eql(u8, prev_parts.base_name, new_parts.base_name) and
797 std.mem.eql(u8, prev_parts.file_ext, new_parts.file_ext))
798 {
799
800 // This is "foo.X.zig" followed by "foo.Y.zig". Make sure that X = Y + 1
801 if (prev_parts.test_index == null) return error.InvalidIncrementalTestIndex;
802 if (new_parts.test_index == null) return error.InvalidIncrementalTestIndex;
803 if (new_parts.test_index.? != prev_parts.test_index.? + 1) return error.InvalidIncrementalTestIndex;
804 } else {
805
806 // This is not the same test sequence, so the new file must be the first file
807 // in a new sequence ("*.1.zig") or an independent test file ("*.zig")
808 if (new_parts.test_index != null and new_parts.test_index.? != 1) return error.InvalidIncrementalTestIndex;
809
810 if (strategy == .independent)
811 opt_case = null; // Generate a new independent test case for this update
812 }
813 }
814 prev_filename = filename;
709815
710 const max_file_size = 10 * 1024 * 1024;816 const max_file_size = 10 * 1024 * 1024;
711 const src = try dir.readFileAllocOptions(ctx.arena, entry.name, max_file_size, null, 1, 0);817 const src = try dir.readFileAllocOptions(ctx.arena, filename, max_file_size, null, 1, 0);
712818
713 // The manifest is the last contiguous block of comments in the file819 // The manifest is the last contiguous block of comments in the file
714 // We scan for the beginning by searching backward for the first non-empty line that does not start with "//"820 // We scan for the beginning by searching backward for the first non-empty line that does not start with "//"
...@@ -741,6 +847,7 @@ pub const TestContext = struct {...@@ -741,6 +847,7 @@ pub const TestContext = struct {
741847
742 if (manifest_start) |start| {848 if (manifest_start) |start| {
743 // Due to the above processing, we know that this is a contiguous block of comments849 // Due to the above processing, we know that this is a contiguous block of comments
850 // and do not need to re-validate the leading "//" on each line
744 var manifest_it = std.mem.tokenize(u8, src[start..manifest_end], "\r\n");851 var manifest_it = std.mem.tokenize(u8, src[start..manifest_end], "\r\n");
745852
746 // First line is the test case name853 // First line is the test case name
...@@ -773,7 +880,6 @@ pub const TestContext = struct {...@@ -773,7 +880,6 @@ pub const TestContext = struct {
773 .independent => {880 .independent => {
774 case.name = case_name;881 case.name = case_name;
775 case.addError(src, errors.items);882 case.addError(src, errors.items);
776 opt_case = null;
777 },883 },
778 .incremental => {884 .incremental => {
779 case.addErrorNamed(case_name, src, errors.items);885 case.addErrorNamed(case_name, src, errors.items);
...@@ -1133,7 +1239,7 @@ pub const TestContext = struct {...@@ -1133,7 +1239,7 @@ pub const TestContext = struct {
1133 if (all_errors.list.len != 0) {1239 if (all_errors.list.len != 0) {
1134 print(1240 print(
1135 "\nCase '{s}': unexpected errors at update_index={d}:\n{s}\n",1241 "\nCase '{s}': unexpected errors at update_index={d}:\n{s}\n",
1136 .{ case.name, update_index, hr },1242 .{ case.name, update_index + 1, hr },
1137 );1243 );
1138 for (all_errors.list) |err_msg| {1244 for (all_errors.list) |err_msg| {
1139 switch (err_msg) {1245 switch (err_msg) {
...@@ -1295,7 +1401,7 @@ pub const TestContext = struct {...@@ -1295,7 +1401,7 @@ pub const TestContext = struct {
1295 }1401 }
12961402
1297 if (any_failed) {1403 if (any_failed) {
1298 print("\nupdate_index={d} ", .{update_index});1404 print("\nupdate_index={d}\n", .{update_index + 1});
1299 return error.WrongCompileErrors;1405 return error.WrongCompileErrors;
1300 }1406 }
1301 },1407 },
...@@ -1402,7 +1508,7 @@ pub const TestContext = struct {...@@ -1402,7 +1508,7 @@ pub const TestContext = struct {
1402 .cwd = tmp_dir_path,1508 .cwd = tmp_dir_path,
1403 }) catch |err| {1509 }) catch |err| {
1404 print("\nupdate_index={d} The following command failed with {s}:\n", .{1510 print("\nupdate_index={d} The following command failed with {s}:\n", .{
1405 update_index, @errorName(err),1511 update_index + 1, @errorName(err),
1406 });1512 });
1407 dumpArgs(argv.items);1513 dumpArgs(argv.items);
1408 return error.ChildProcessExecution;1514 return error.ChildProcessExecution;