From b2427ea7d839a9e17568206c64da56865cd000f1 Mon Sep 17 00:00:00 2001 From: february cozzocrea Date: Mon, 4 Mar 2024 17:26:41 -0800 Subject: [PATCH] test manifest key checking and other fixes This commit adds several fixes and improvements for the Zig compiler test harness. 1. -Dskip-translate-c option added for skipping the translate-c tests. 2. translate-c/run-translated-c tests in test/cases/* have been added to the steps test-translate-c and test-run-translated-c. Closes #18224. 3. Custom name added to the CheckFile step for the translate-c step to better communicate which test failed. 4. Test manifest key validation added to return an error if a manifest contains an invalid key. --- build.zig | 11 +- ...compound_assignments_with_implicit_casts.c | 2 +- .../explicit_cast_bool_from_float.c | 2 +- test/src/Cases.zig | 162 +++++++++++------- test/tests.zig | 17 +- 5 files changed, 120 insertions(+), 74 deletions(-) diff --git a/build.zig b/build.zig index ad263a9584..bacedfbcd1 100644 --- a/build.zig +++ b/build.zig @@ -101,6 +101,7 @@ pub fn build(b: *std.Build) !void { const skip_non_native = b.option(bool, "skip-non-native", "Main test suite skips non-native builds") orelse false; const skip_libc = b.option(bool, "skip-libc", "Main test suite skips tests that link libc") orelse false; const skip_single_threaded = b.option(bool, "skip-single-threaded", "Main test suite skips tests that are single-threaded") orelse false; + const skip_translate_c = b.option(bool, "skip-translate-c", "Main test suite skips translate-c tests") orelse false; const skip_run_translated_c = b.option(bool, "skip-run-translated-c", "Main test suite skips run-translated-c tests") orelse false; const only_install_lib_files = b.option(bool, "lib-files-only", "Only install library files") orelse false; @@ -453,7 +454,10 @@ pub fn build(b: *std.Build) !void { }).step); const test_cases_step = b.step("test-cases", "Run the main compiler test cases"); - try tests.addCases(b, test_cases_step, test_filters, check_case_exe, .{ + try tests.addCases(b, test_cases_step, test_filters, check_case_exe, target, .{ + .skip_translate_c = skip_translate_c, + .skip_run_translated_c = skip_run_translated_c, + }, .{ .enable_llvm = enable_llvm, .llvm_has_m68k = llvm_has_m68k, .llvm_has_csky = llvm_has_csky, @@ -525,11 +529,6 @@ pub fn build(b: *std.Build) !void { test_step.dependOn(tests.addStackTraceTests(b, test_filters, optimization_modes)); test_step.dependOn(tests.addCliTests(b)); test_step.dependOn(tests.addAssembleAndLinkTests(b, test_filters, optimization_modes)); - test_step.dependOn(tests.addTranslateCTests(b, test_filters)); - if (!skip_run_translated_c) { - test_step.dependOn(tests.addRunTranslatedCTests(b, test_filters, target)); - } - test_step.dependOn(tests.addModuleTests(b, .{ .test_filters = test_filters, .root_src = "lib/std/std.zig", diff --git a/test/cases/run_translated_c/compound_assignments_with_implicit_casts.c b/test/cases/run_translated_c/compound_assignments_with_implicit_casts.c index c1cadfd925..79029d4903 100644 --- a/test/cases/run_translated_c/compound_assignments_with_implicit_casts.c +++ b/test/cases/run_translated_c/compound_assignments_with_implicit_casts.c @@ -17,4 +17,4 @@ int main() { } // run-translated-c -// c_frontends=aro,clang +// c_frontend=clang diff --git a/test/cases/run_translated_c/explicit_cast_bool_from_float.c b/test/cases/run_translated_c/explicit_cast_bool_from_float.c index c8e8ea7e49..79bb88cc4b 100644 --- a/test/cases/run_translated_c/explicit_cast_bool_from_float.c +++ b/test/cases/run_translated_c/explicit_cast_bool_from_float.c @@ -7,4 +7,4 @@ int main() { } // run-translated-c -// c_frontends=aro,clang +// c_frontend=clang diff --git a/test/src/Cases.zig b/test/src/Cases.zig index 58e41fc4fe..df512e91d0 100644 --- a/test/src/Cases.zig +++ b/test/src/Cases.zig @@ -533,6 +533,98 @@ pub fn init(gpa: Allocator, arena: Allocator) Cases { }; } +pub const TranslateCOptions = struct { + skip_translate_c: bool = false, + skip_run_translated_c: bool = false, +}; +pub fn lowerToTranslateCSteps( + self: *Cases, + b: *std.Build, + parent_step: *std.Build.Step, + test_filters: []const []const u8, + target: std.Build.ResolvedTarget, + translate_c_options: TranslateCOptions, +) void { + const host = std.zig.system.resolveTargetQuery(.{}) catch |err| + std.debug.panic("unable to detect native host: {s}\n", .{@errorName(err)}); + + const tests = @import("../tests.zig"); + const test_translate_c_step = b.step("test-translate-c", "Run the C translation tests"); + if (!translate_c_options.skip_translate_c) { + tests.addTranslateCTests(b, test_translate_c_step, test_filters); + parent_step.dependOn(test_translate_c_step); + } + + const test_run_translated_c_step = b.step("test-run-translated-c", "Run the Run-Translated-C tests"); + if (!translate_c_options.skip_run_translated_c) { + tests.addRunTranslatedCTests(b, test_run_translated_c_step, test_filters, target); + parent_step.dependOn(test_run_translated_c_step); + } + + for (self.translate.items) |case| switch (case.kind) { + .run => |output| { + if (translate_c_options.skip_run_translated_c) continue; + const annotated_case_name = b.fmt("run-translated-c {s}", .{case.name}); + for (test_filters) |test_filter| { + if (std.mem.indexOf(u8, annotated_case_name, test_filter)) |_| break; + } else if (test_filters.len > 0) continue; + if (!std.process.can_spawn) { + std.debug.print("Unable to spawn child processes on {s}, skipping test.\n", .{@tagName(builtin.os.tag)}); + continue; // Pass test. + } + + if (getExternalExecutor(host, &case.target.result, .{ .link_libc = true }) != .native) { + // We wouldn't be able to run the compiled C code. + continue; // Pass test. + } + + const write_src = b.addWriteFiles(); + const file_source = write_src.add("tmp.c", case.input); + + const translate_c = b.addTranslateC(.{ + .root_source_file = file_source, + .optimize = .Debug, + .target = case.target, + .link_libc = case.link_libc, + .use_clang = case.c_frontend == .clang, + }); + translate_c.step.name = b.fmt("{s} translate-c", .{annotated_case_name}); + + const run_exe = translate_c.addExecutable(.{}); + run_exe.step.name = b.fmt("{s} build-exe", .{annotated_case_name}); + run_exe.linkLibC(); + const run = b.addRunArtifact(run_exe); + run.step.name = b.fmt("{s} run", .{annotated_case_name}); + run.expectStdOutEqual(output); + + test_run_translated_c_step.dependOn(&run.step); + }, + .translate => |output| { + if (translate_c_options.skip_translate_c) continue; + const annotated_case_name = b.fmt("zig translate-c {s}", .{case.name}); + for (test_filters) |test_filter| { + if (std.mem.indexOf(u8, annotated_case_name, test_filter)) |_| break; + } else if (test_filters.len > 0) continue; + + const write_src = b.addWriteFiles(); + const file_source = write_src.add("tmp.c", case.input); + + const translate_c = b.addTranslateC(.{ + .root_source_file = file_source, + .optimize = .Debug, + .target = case.target, + .link_libc = case.link_libc, + .use_clang = case.c_frontend == .clang, + }); + translate_c.step.name = b.fmt("{s} translate-c", .{annotated_case_name}); + + const check_file = translate_c.addCheckFile(output); + check_file.step.name = b.fmt("{s} CheckFile", .{annotated_case_name}); + test_translate_c_step.dependOn(&check_file.step); + }, + }; +} + pub fn lowerToBuildSteps( self: *Cases, b: *std.Build, @@ -681,66 +773,6 @@ pub fn lowerToBuildSteps( .Header => @panic("TODO"), } } - - for (self.translate.items) |case| switch (case.kind) { - .run => |output| { - const annotated_case_name = b.fmt("run-translated-c {s}", .{case.name}); - for (test_filters) |test_filter| { - if (std.mem.indexOf(u8, annotated_case_name, test_filter)) |_| break; - } else if (test_filters.len > 0) continue; - if (!std.process.can_spawn) { - std.debug.print("Unable to spawn child processes on {s}, skipping test.\n", .{@tagName(builtin.os.tag)}); - continue; // Pass test. - } - - if (getExternalExecutor(host, &case.target.result, .{ .link_libc = true }) != .native) { - // We wouldn't be able to run the compiled C code. - continue; // Pass test. - } - - const write_src = b.addWriteFiles(); - const file_source = write_src.add("tmp.c", case.input); - - const translate_c = b.addTranslateC(.{ - .root_source_file = file_source, - .optimize = .Debug, - .target = case.target, - .link_libc = case.link_libc, - .use_clang = case.c_frontend == .clang, - }); - translate_c.step.name = b.fmt("{s} translate-c", .{annotated_case_name}); - - const run_exe = translate_c.addExecutable(.{}); - run_exe.step.name = b.fmt("{s} build-exe", .{annotated_case_name}); - run_exe.linkLibC(); - const run = b.addRunArtifact(run_exe); - run.step.name = b.fmt("{s} run", .{annotated_case_name}); - run.expectStdOutEqual(output); - - parent_step.dependOn(&run.step); - }, - .translate => |output| { - const annotated_case_name = b.fmt("zig translate-c {s}", .{case.name}); - for (test_filters) |test_filter| { - if (std.mem.indexOf(u8, annotated_case_name, test_filter)) |_| break; - } else if (test_filters.len > 0) continue; - - const write_src = b.addWriteFiles(); - const file_source = write_src.add("tmp.c", case.input); - - const translate_c = b.addTranslateC(.{ - .root_source_file = file_source, - .optimize = .Debug, - .target = case.target, - .link_libc = case.link_libc, - .use_clang = case.c_frontend == .clang, - }); - translate_c.step.name = annotated_case_name; - - const check_file = translate_c.addCheckFile(output); - parent_step.dependOn(&check_file.step); - }, - }; } /// Sort test filenames in-place, so that incremental test cases ("foo.0.zig", @@ -961,6 +993,15 @@ const TestManifest = struct { config_map: std.StringHashMap([]const u8), trailing_bytes: []const u8 = "", + const valid_keys = std.ComptimeStringMap(void, .{ + .{ "is_test", {} }, + .{ "output_mode", {} }, + .{ "target", {} }, + .{ "c_frontend", {} }, + .{ "link_libc", {} }, + .{ "backend", {} }, + }); + const Type = enum { @"error", run, @@ -1059,6 +1100,7 @@ const TestManifest = struct { // Parse key=value(s) var kv_it = std.mem.splitScalar(u8, trimmed, '='); const key = kv_it.first(); + if (!valid_keys.has(key)) return error.InvalidKey; try manifest.config_map.putNoClobber(key, kv_it.next() orelse return error.MissingValuesForConfig); } diff --git a/test/tests.zig b/test/tests.zig index cbee78b2bd..525c6792b5 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -998,29 +998,30 @@ pub fn addAssembleAndLinkTests(b: *std.Build, test_filters: []const []const u8, return cases.step; } -pub fn addTranslateCTests(b: *std.Build, test_filters: []const []const u8) *Step { +pub fn addTranslateCTests(b: *std.Build, parent_step: *std.Build.Step, test_filters: []const []const u8) void { const cases = b.allocator.create(TranslateCContext) catch @panic("OOM"); cases.* = TranslateCContext{ .b = b, - .step = b.step("test-translate-c", "Run the C translation tests"), + .step = parent_step, .test_index = 0, .test_filters = test_filters, }; translate_c.addCases(cases); - return cases.step; + return; } pub fn addRunTranslatedCTests( b: *std.Build, + parent_step: *std.Build.Step, test_filters: []const []const u8, target: std.Build.ResolvedTarget, -) *Step { +) void { const cases = b.allocator.create(RunTranslatedCContext) catch @panic("OOM"); cases.* = .{ .b = b, - .step = b.step("test-run-translated-c", "Run the Run-Translated-C tests"), + .step = parent_step, .test_index = 0, .test_filters = test_filters, .target = target, @@ -1028,7 +1029,7 @@ pub fn addRunTranslatedCTests( run_translated_c.addCases(cases); - return cases.step; + return; } const ModuleTestOptions = struct { @@ -1288,6 +1289,8 @@ pub fn addCases( parent_step: *Step, test_filters: []const []const u8, check_case_exe: *std.Build.Step.Compile, + target: std.Build.ResolvedTarget, + translate_c_options: @import("src/Cases.zig").TranslateCOptions, build_options: @import("cases.zig").BuildOptions, ) !void { const arena = b.allocator; @@ -1301,6 +1304,8 @@ pub fn addCases( cases.addFromDir(dir, b); try @import("cases.zig").addCases(&cases, build_options, b); + cases.lowerToTranslateCSteps(b, parent_step, test_filters, target, translate_c_options); + const cases_dir_path = try b.build_root.join(b.allocator, &.{ "test", "cases" }); cases.lowerToBuildSteps( b,