authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2022-05-03 10:29:12+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2022-05-04 23:21:35+02:00
log9d79b740bc981da9f336d5c72d1abe7d81e6ad9f
tree3c5a196d29a15c368552c593087578f58480ff4b
parenta839ccc15322e63b13083da8f44959113a5e3313

test: improve test batch/sequence iterator

With this improved iterator, type of test is now inferred from the filename, enabling us to put all cases in one common parent directory, and iterate over that, thus automating a lot of tasks.

1 files changed, 205 insertions(+), 165 deletions(-)

src/test.zig+205-165
...@@ -44,7 +44,7 @@ test {...@@ -44,7 +44,7 @@ test {
4444
45 // TODO make this incremental once the bug is solved that it triggers45 // TODO make this incremental once the bug is solved that it triggers
46 // See: https://github.com/ziglang/zig/issues/1134446 // See: https://github.com/ziglang/zig/issues/11344
47 ctx.addTestCasesFromDir(dir, .independent);47 ctx.addTestCasesFromDir(dir);
48 }48 }
4949
50 {50 {
...@@ -55,7 +55,7 @@ test {...@@ -55,7 +55,7 @@ test {
55 var dir = try std.fs.cwd().openDir(dir_path, .{ .iterate = true });55 var dir = try std.fs.cwd().openDir(dir_path, .{ .iterate = true });
56 defer dir.close();56 defer dir.close();
5757
58 ctx.addTestCasesFromDir(dir, .incremental);58 ctx.addTestCasesFromDir(dir);
59 }59 }
6060
61 try @import("test_cases").addCases(&ctx);61 try @import("test_cases").addCases(&ctx);
...@@ -395,6 +395,134 @@ const TestManifest = struct {...@@ -395,6 +395,134 @@ const TestManifest = struct {
395 }395 }
396};396};
397397
398const TestStrategy = enum {
399 /// Execute tests as independent compilations, unless they are explicitly
400 /// incremental ("foo.0.zig", "foo.1.zig", etc.)
401 independent,
402 /// Execute all tests as incremental updates to a single compilation. Explicitly
403 /// incremental tests ("foo.0.zig", "foo.1.zig", etc.) still execute in order
404 incremental,
405};
406
407/// Iterates a set of filenames extracting batches that are either incremental
408/// ("foo.0.zig", "foo.1.zig", etc.) or independent ("foo.zig", "bar.zig", etc.).
409/// Assumes filenames are sorted.
410const TestIterator = struct {
411 start: usize = 0,
412 end: usize = 0,
413 filenames: []const []const u8,
414
415 const Error = error{InvalidIncrementalTestIndex};
416
417 fn next(it: *TestIterator) Error!?[]const []const u8 {
418 try it.nextInner();
419 if (it.start == it.end) return null;
420 return it.filenames[it.start..it.end];
421 }
422
423 fn nextInner(it: *TestIterator) Error!void {
424 it.start = it.end;
425 if (it.end == it.filenames.len) return;
426 if (it.end + 1 == it.filenames.len) {
427 it.end += 1;
428 return;
429 }
430
431 const remaining = it.filenames[it.end..];
432 var i: usize = 0;
433 while (i < remaining.len - 1) : (i += 1) {
434 // First, check if this file is part of an incremental update sequence
435 // Split filename into "<base_name>.<index>.<file_ext>"
436 const prev_parts = getTestFileNameParts(remaining[i]);
437 const new_parts = getTestFileNameParts(remaining[i + 1]);
438
439 // If base_name and file_ext match, these files are in the same test sequence
440 // and the new one should be the incremented version of the previous test
441 if (std.mem.eql(u8, prev_parts.base_name, new_parts.base_name) and
442 std.mem.eql(u8, prev_parts.file_ext, new_parts.file_ext))
443 {
444 // This is "foo.X.zig" followed by "foo.Y.zig". Make sure that X = Y + 1
445 if (prev_parts.test_index == null)
446 return error.InvalidIncrementalTestIndex;
447 if (new_parts.test_index == null)
448 return error.InvalidIncrementalTestIndex;
449 if (new_parts.test_index.? != prev_parts.test_index.? + 1)
450 return error.InvalidIncrementalTestIndex;
451 } else {
452 // This is not the same test sequence, so the new file must be the first file
453 // in a new sequence ("*.0.zig") or an independent test file ("*.zig")
454 if (new_parts.test_index != null and new_parts.test_index.? != 0)
455 return error.InvalidIncrementalTestIndex;
456
457 it.end += i + 1;
458 break;
459 }
460 } else {
461 it.end += remaining.len;
462 }
463 }
464};
465
466/// For a filename in the format "<filename>.X.<ext>" or "<filename>.<ext>", returns
467/// "<filename>", "<ext>" and X parsed as a decimal number. If X is not present, or
468/// cannot be parsed as a decimal number, it is treated as part of <filename>
469fn getTestFileNameParts(name: []const u8) struct {
470 base_name: []const u8,
471 file_ext: []const u8,
472 test_index: ?usize,
473} {
474 const file_ext = std.fs.path.extension(name);
475 const trimmed = name[0 .. name.len - file_ext.len]; // Trim off ".<ext>"
476 const maybe_index = std.fs.path.extension(trimmed); // Extract ".X"
477
478 // Attempt to parse index
479 const index: ?usize = if (maybe_index.len > 0)
480 std.fmt.parseInt(usize, maybe_index[1..], 10) catch null
481 else
482 null;
483
484 // Adjust "<filename>" extent based on parsing success
485 const base_name_end = trimmed.len - if (index != null) maybe_index.len else 0;
486 return .{
487 .base_name = name[0..base_name_end],
488 .file_ext = if (file_ext.len > 0) file_ext[1..] else file_ext,
489 .test_index = index,
490 };
491}
492
493/// Sort test filenames in-place, so that incremental test cases ("foo.0.zig",
494/// "foo.1.zig", etc.) are contiguous and appear in numerical order.
495fn sortTestFilenames(
496 filenames: [][]const u8,
497) void {
498 const Context = struct {
499 pub fn lessThan(_: @This(), a: []const u8, b: []const u8) bool {
500 const a_parts = getTestFileNameParts(a);
501 const b_parts = getTestFileNameParts(b);
502
503 // Sort "<base_name>.X.<file_ext>" based on "<base_name>" and "<file_ext>" first
504 return switch (std.mem.order(u8, a_parts.base_name, b_parts.base_name)) {
505 .lt => true,
506 .gt => false,
507 .eq => switch (std.mem.order(u8, a_parts.file_ext, b_parts.file_ext)) {
508 .lt => true,
509 .gt => false,
510 .eq => b: { // a and b differ only in their ".X" part
511
512 // Sort "<base_name>.<file_ext>" before any "<base_name>.X.<file_ext>"
513 if (a_parts.test_index == null) break :b true;
514 if (b_parts.test_index == null) break :b false;
515
516 // Make sure that incremental tests appear in linear order
517 return a_parts.test_index.? < b_parts.test_index.?;
518 },
519 },
520 };
521 }
522 };
523 std.sort.sort([]const u8, filenames, Context{}, Context.lessThan);
524}
525
398pub const TestContext = struct {526pub const TestContext = struct {
399 arena: Allocator,527 arena: Allocator,
400 cases: std.ArrayList(Case),528 cases: std.ArrayList(Case),
...@@ -895,100 +1023,29 @@ pub const TestContext = struct {...@@ -895,100 +1023,29 @@ pub const TestContext = struct {
895 case.compiles(fixed_src);1023 case.compiles(fixed_src);
896 }1024 }
8971025
898 const Strategy = enum {1026 /// Adds a test for each file in the provided directory.
899 /// Execute tests as independent compilations, unless they are explicitly1027 /// Testing strategy (TestStrategy) is inferred automatically from filenames.
900 /// incremental ("foo.0.zig", "foo.1.zig", etc.)
901 independent,
902 /// Execute all tests as incremental updates to a single compilation. Explicitly
903 /// incremental tests ("foo.0.zig", "foo.1.zig", etc.) still execute in order
904 incremental,
905 };
906
907 /// Adds a test for each file in the provided directory, using the selected strategy.
908 /// Recurses nested directories.1028 /// Recurses nested directories.
909 ///1029 ///
910 /// Each file should include a test manifest as a contiguous block of comments at1030 /// Each file should include a test manifest as a contiguous block of comments at
911 /// the end of the file. The first line should be the test type, followed by a set of1031 /// the end of the file. The first line should be the test type, followed by a set of
912 /// key-value config values, followed by a blank line, then the expected output.1032 /// key-value config values, followed by a blank line, then the expected output.
913 pub fn addTestCasesFromDir(ctx: *TestContext, dir: std.fs.Dir, strategy: Strategy) void {1033 pub fn addTestCasesFromDir(ctx: *TestContext, dir: std.fs.Dir) void {
914 var current_file: []const u8 = "none";1034 var current_file: []const u8 = "none";
915 ctx.addTestCasesFromDirInner(dir, strategy, &current_file) catch |err| {1035 ctx.addTestCasesFromDirInner(dir, &current_file) catch |err| {
916 std.debug.panic("test harness failed to process file '{s}': {s}\n", .{1036 std.debug.panic("test harness failed to process file '{s}': {s}\n", .{
917 current_file, @errorName(err),1037 current_file, @errorName(err),
918 });1038 });
919 };1039 };
920 }1040 }
9211041
922 /// For a filename in the format "<filename>.X.<ext>" or "<filename>.<ext>", returns
923 /// "<filename>", "<ext>" and X parsed as a decimal number. If X is not present, or
924 /// cannot be parsed as a decimal number, it is treated as part of <filename>
925 fn getTestFileNameParts(name: []const u8) struct {
926 base_name: []const u8,
927 file_ext: []const u8,
928 test_index: ?usize,
929 } {
930 const file_ext = std.fs.path.extension(name);
931 const trimmed = name[0 .. name.len - file_ext.len]; // Trim off ".<ext>"
932 const maybe_index = std.fs.path.extension(trimmed); // Extract ".X"
933
934 // Attempt to parse index
935 const index: ?usize = if (maybe_index.len > 0)
936 std.fmt.parseInt(usize, maybe_index[1..], 10) catch null
937 else
938 null;
939
940 // Adjust "<filename>" extent based on parsing success
941 const base_name_end = trimmed.len - if (index != null) maybe_index.len else 0;
942 return .{
943 .base_name = name[0..base_name_end],
944 .file_ext = if (file_ext.len > 0) file_ext[1..] else file_ext,
945 .test_index = index,
946 };
947 }
948
949 /// Sort test filenames in-place, so that incremental test cases ("foo.0.zig",
950 /// "foo.1.zig", etc.) are contiguous and appear in numerical order.
951 fn sortTestFilenames(
952 filenames: [][]const u8,
953 ) void {
954 const Context = struct {
955 pub fn lessThan(_: @This(), a: []const u8, b: []const u8) bool {
956 const a_parts = getTestFileNameParts(a);
957 const b_parts = getTestFileNameParts(b);
958
959 // Sort "<base_name>.X.<file_ext>" based on "<base_name>" and "<file_ext>" first
960 return switch (std.mem.order(u8, a_parts.base_name, b_parts.base_name)) {
961 .lt => true,
962 .gt => false,
963 .eq => switch (std.mem.order(u8, a_parts.file_ext, b_parts.file_ext)) {
964 .lt => true,
965 .gt => false,
966 .eq => b: { // a and b differ only in their ".X" part
967
968 // Sort "<base_name>.<file_ext>" before any "<base_name>.X.<file_ext>"
969 if (a_parts.test_index == null) break :b true;
970 if (b_parts.test_index == null) break :b false;
971
972 // Make sure that incremental tests appear in linear order
973 return a_parts.test_index.? < b_parts.test_index.?;
974 },
975 },
976 };
977 }
978 };
979 std.sort.sort([]const u8, filenames, Context{}, Context.lessThan);
980 }
981
982 fn addTestCasesFromDirInner(1042 fn addTestCasesFromDirInner(
983 ctx: *TestContext,1043 ctx: *TestContext,
984 dir: std.fs.Dir,1044 dir: std.fs.Dir,
985 strategy: Strategy,
986 /// This is kept up to date with the currently being processed file so1045 /// This is kept up to date with the currently being processed file so
987 /// that if any errors occur the caller knows it happened during this file.1046 /// that if any errors occur the caller knows it happened during this file.
988 current_file: *[]const u8,1047 current_file: *[]const u8,
989 ) !void {1048 ) !void {
990 var cases = std.ArrayList(usize).init(ctx.arena);
991
992 var it = try dir.walk(ctx.arena);1049 var it = try dir.walk(ctx.arena);
993 var filenames = std.ArrayList([]const u8).init(ctx.arena);1050 var filenames = std.ArrayList([]const u8).init(ctx.arena);
9941051
...@@ -1006,104 +1063,87 @@ pub const TestContext = struct {...@@ -1006,104 +1063,87 @@ pub const TestContext = struct {
1006 // Sort filenames, so that incremental tests are contiguous and in-order1063 // Sort filenames, so that incremental tests are contiguous and in-order
1007 sortTestFilenames(filenames.items);1064 sortTestFilenames(filenames.items);
10081065
1009 var prev_filename: []const u8 = "";1066 var test_it = TestIterator{ .filenames = filenames.items };
1010 for (filenames.items) |filename| {1067 while (try test_it.next()) |batch| {
1011 current_file.* = filename;1068 const strategy: TestStrategy = if (batch.len > 1) .incremental else .independent;
1069 var cases = std.ArrayList(usize).init(ctx.arena);
10121070
1013 // First, check if this file is part of an incremental update sequence1071 for (batch) |filename| {
1014 // Split filename into "<base_name>.<index>.<file_ext>"1072 current_file.* = filename;
1015 const prev_parts = getTestFileNameParts(prev_filename);
1016 const new_parts = getTestFileNameParts(filename);
10171073
1018 // If base_name and file_ext match, these files are in the same test sequence1074 const max_file_size = 10 * 1024 * 1024;
1019 // and the new one should be the incremented version of the previous test1075 const src = try dir.readFileAllocOptions(ctx.arena, filename, max_file_size, null, 1, 0);
1020 if (std.mem.eql(u8, prev_parts.base_name, new_parts.base_name) and
1021 std.mem.eql(u8, prev_parts.file_ext, new_parts.file_ext))
1022 {
1023 // This is "foo.X.zig" followed by "foo.Y.zig". Make sure that X = Y + 1
1024 if (prev_parts.test_index == null) return error.InvalidIncrementalTestIndex;
1025 if (new_parts.test_index == null) return error.InvalidIncrementalTestIndex;
1026 if (new_parts.test_index.? != prev_parts.test_index.? + 1) return error.InvalidIncrementalTestIndex;
1027 } else {
1028 // This is not the same test sequence, so the new file must be the first file
1029 // in a new sequence ("*.0.zig") or an independent test file ("*.zig")
1030 if (new_parts.test_index != null and new_parts.test_index.? != 0) return error.InvalidIncrementalTestIndex;
1031 cases.clearRetainingCapacity();
1032 }
1033 prev_filename = filename;
10341076
1035 const max_file_size = 10 * 1024 * 1024;1077 // Parse the manifest
1036 const src = try dir.readFileAllocOptions(ctx.arena, filename, max_file_size, null, 1, 0);1078 var manifest = try TestManifest.parse(ctx.arena, src);
10371079
1038 // Parse the manifest1080 if (cases.items.len == 0) {
1039 var manifest = try TestManifest.parse(ctx.arena, src);1081 const backends = try manifest.getConfigForKeyAlloc(ctx.arena, "backend", Backend);
1082 const targets = try manifest.getConfigForKeyAlloc(ctx.arena, "target", CrossTarget);
1083 const is_test = manifest.getConfigForKeyAssertSingle("is_test", bool);
1084 const output_mode = manifest.getConfigForKeyAssertSingle("output_mode", std.builtin.OutputMode);
10401085
1041 if (cases.items.len == 0) {1086 const name_prefix = blk: {
1042 const backends = try manifest.getConfigForKeyAlloc(ctx.arena, "backend", Backend);1087 const ext_index = std.mem.lastIndexOfScalar(u8, current_file.*, '.') orelse
1043 const targets = try manifest.getConfigForKeyAlloc(ctx.arena, "target", CrossTarget);1088 return error.InvalidFilename;
1044 const is_test = manifest.getConfigForKeyAssertSingle("is_test", bool);1089 const index = std.mem.lastIndexOfScalar(u8, current_file.*[0..ext_index], '.') orelse ext_index;
1045 const output_mode = manifest.getConfigForKeyAssertSingle("output_mode", std.builtin.OutputMode);1090 break :blk current_file.*[0..index];
1091 };
10461092
1047 const name_prefix = blk: {1093 // Cross-product to get all possible test combinations
1048 const ext_index = std.mem.lastIndexOfScalar(u8, current_file.*, '.') orelse1094 for (backends) |backend| {
1049 return error.InvalidFilename;1095 if (backend == .stage1 and skip_stage1) continue;
1050 const index = std.mem.lastIndexOfScalar(u8, current_file.*[0..ext_index], '.') orelse ext_index;
1051 break :blk current_file.*[0..index];
1052 };
10531096
1054 // Cross-product to get all possible test combinations1097 for (targets) |target| {
1055 for (backends) |backend| {1098 const name = try std.fmt.allocPrint(ctx.arena, "{s} ({s}, {s})", .{
1056 if (backend == .stage1 and skip_stage1) continue;1099 name_prefix,
10571100 @tagName(backend),
1058 for (targets) |target| {1101 try target.zigTriple(ctx.arena),
1059 const name = try std.fmt.allocPrint(ctx.arena, "{s} ({s}, {s})", .{1102 });
1060 name_prefix,1103 const next = ctx.cases.items.len;
1061 @tagName(backend),1104 try ctx.cases.append(.{
1062 try target.zigTriple(ctx.arena),1105 .name = name,
1063 });1106 .target = target,
1064 const next = ctx.cases.items.len;1107 .backend = backend,
1065 try ctx.cases.append(.{1108 .updates = std.ArrayList(TestContext.Update).init(ctx.cases.allocator),
1066 .name = name,1109 .is_test = is_test,
1067 .target = target,1110 .output_mode = output_mode,
1068 .backend = backend,1111 .link_libc = backend == .llvm,
1069 .updates = std.ArrayList(TestContext.Update).init(ctx.cases.allocator),1112 .files = std.ArrayList(TestContext.File).init(ctx.cases.allocator),
1070 .is_test = is_test,1113 });
1071 .output_mode = output_mode,1114 try cases.append(next);
1072 .link_libc = backend == .llvm,1115 }
1073 .files = std.ArrayList(TestContext.File).init(ctx.cases.allocator),
1074 });
1075 try cases.append(next);
1076 }1116 }
1077 }1117 }
1078 }
10791118
1080 for (cases.items) |case_index| {1119 for (cases.items) |case_index| {
1081 const case = &ctx.cases.items[case_index];1120 const case = &ctx.cases.items[case_index];
1082 switch (manifest.@"type") {1121 switch (manifest.@"type") {
1083 .@"error" => {1122 .@"error" => {
1084 const errors = try manifest.trailingAlloc(ctx.arena);1123 const errors = try manifest.trailingAlloc(ctx.arena);
1085 switch (strategy) {1124 switch (strategy) {
1086 .independent => {1125 .independent => {
1087 case.addError(src, errors);1126 case.addError(src, errors);
1088 },1127 },
1089 .incremental => {1128 .incremental => {
1090 case.addErrorNamed("update", src, errors);1129 case.addErrorNamed("update", src, errors);
1091 },1130 },
1092 }1131 }
1093 },1132 },
1094 .run => {1133 .run => {
1095 var output = std.ArrayList(u8).init(ctx.arena);1134 var output = std.ArrayList(u8).init(ctx.arena);
1096 var trailing_it = manifest.trailing();1135 var trailing_it = manifest.trailing();
1097 while (trailing_it.next()) |line| {1136 while (trailing_it.next()) |line| {
1098 try output.appendSlice(line);1137 try output.appendSlice(line);
1099 try output.append('\n');1138 try output.append('\n');
1100 }1139 }
1101 if (output.items.len > 0) {1140 if (output.items.len > 0) {
1102 try output.resize(output.items.len - 1);1141 try output.resize(output.items.len - 1);
1103 }1142 }
1104 case.addCompareOutput(src, output.toOwnedSlice());1143 case.addCompareOutput(src, output.toOwnedSlice());
1105 },1144 },
1106 .cli => @panic("TODO cli tests"),1145 .cli => @panic("TODO cli tests"),
1146 }
1107 }1147 }
1108 }1148 }
1109 }1149 }