From d81118769aac21b065edb51f03073d2c66617644 Mon Sep 17 00:00:00 2001 From: Tom Read Cutting Date: Fri, 19 Aug 2022 19:36:28 +0100 Subject: [PATCH 1/2] Add tests for confirming bad arguments are handled correctly --- src/archive/Archive.zig | 2 +- src/main.zig | 12 +++-- src/test.zig | 99 +++++++++++++++++++++++++++++++++-------- 3 files changed, 90 insertions(+), 23 deletions(-) diff --git a/src/archive/Archive.zig b/src/archive/Archive.zig index 9369077..d4c554c 100644 --- a/src/archive/Archive.zig +++ b/src/archive/Archive.zig @@ -211,7 +211,7 @@ pub const Contents = struct { uid: u32, gid: u32, - // TODO: dellocation + // TODO: deallocation pub fn write(self: *const Contents, out_stream: anytype, stderr: anytype) !void { try out_stream.writeAll(self.bytes); diff --git a/src/main.zig b/src/main.zig index 50dd44e..556c8e6 100644 --- a/src/main.zig +++ b/src/main.zig @@ -9,7 +9,7 @@ const process = std.process; const Archive = @import("archive/Archive.zig"); -const overview = +pub const overview = \\Zig Archiver \\ \\Usage: zar [options] [-][modifiers] [relpos] [count] [files] @@ -52,6 +52,8 @@ const overview = \\ ; +pub const error_prefix = overview ++ "\n\x1B[1;31merror\x1B[0m: "; + const version = "0.0.0"; const version_details = @@ -94,7 +96,7 @@ pub fn log( // through to the program be the user, we do this often enough that it's worth // having a procedure for it. fn printArgumentError(comptime errorString: []const u8, args: anytype) void { - logger.err(overview ++ "\nShowing help text above as error occured:\n" ++ errorString, args); + logger.err(error_prefix ++ errorString, args); } fn checkArgsBounds(args: []const []const u8, index: u32, comptime missing_argument: []const u8) bool { @@ -278,9 +280,11 @@ pub fn archiveMain(cwd: fs.Dir, allocator: anytype, args: []const []const u8) an 'R' => modifiers.sort_symbol_table = .set_false, 'a' => modifiers.move_setting = .before, 'b', 'i' => modifiers.move_setting = .after, - // TODO: should we print warning with unknown modifier? // TODO: handle other modifiers! - else => {}, + else => { + printArgumentError("'{c}' is not a valid modifier.", .{modifier_char}); + return; + }, } } } diff --git a/src/test.zig b/src/test.zig index b8fece2..9dc97d2 100644 --- a/src/test.zig +++ b/src/test.zig @@ -11,6 +11,8 @@ const Allocator = std.mem.Allocator; const Archive = @import("archive/Archive.zig"); const main = @import("main.zig"); +const path_to_zar = "../../../zig-out/bin/zar"; + const llvm_ar_archive_name = "llvm-ar-archive.a"; const zig_ar_archive_name = "zig-ar-archive.a"; @@ -38,7 +40,7 @@ const no_dir = "test/data/none"; // Allows us to invoke zar as a program, just to really confirm it works // end-to-end. -const invoke_zar_as_child_process = false; +const always_invoke_zar_as_child_process = false; test "Test Archive Text Basic" { const test1_dir = "test/data/test1"; @@ -118,6 +120,45 @@ test "Test Archive Sorted" { try doStandardTests(arena.allocator(), no_dir, &test_sort_names, &test_sort); } +test "Test Argument Errors" { + const allocator = std.testing.allocator; + var test_dir_info = try TestDirInfo.getInfo(); + defer test_dir_info.cleanup(); + + var argv = std.ArrayList([]const u8).init(allocator); + defer argv.deinit(); + try argv.append(path_to_zar); + + { + try argv.resize(1); + const expected_out: ExpectedOut = .{ + .stderr = "error(archive_main): " ++ main.error_prefix ++ "An operation must be provided.\n", + }; + + try invokeZar(allocator, argv.items, test_dir_info, expected_out); + } + + { + try argv.resize(1); + try argv.append("j"); + const expected_out: ExpectedOut = .{ + .stderr = "error(archive_main): " ++ main.error_prefix ++ "'j' is not a valid operation.\n", + }; + + try invokeZar(allocator, argv.items, test_dir_info, expected_out); + } + + { + try argv.resize(1); + try argv.append("rj"); + const expected_out: ExpectedOut = .{ + .stderr = "error(archive_main): " ++ main.error_prefix ++ "'j' is not a valid modifier.\n", + }; + + try invokeZar(allocator, argv.items, test_dir_info, expected_out); + } +} + fn initialiseTestData(allocator: Allocator, file_names: [][]const u8, symbol_names: [][][]const u8, symbol_count: u32) !void { for (file_names) |_, index| { file_names[index] = try std.fmt.allocPrint(allocator, "index_{}.o", .{index}); @@ -419,25 +460,22 @@ fn copyAssetsToTestDirectory(comptime test_src_dir_path: []const u8, file_names: } } -fn doZarArchiveOperation(comptime format: LlvmFormat, comptime operation: []const u8, file_names: []const []const u8, test_dir_info: TestDirInfo) !void { - const tracy = trace(@src()); - defer tracy.end(); - const allocator = std.testing.allocator; - - var argv = std.ArrayList([]const u8).init(allocator); - defer argv.deinit(); - - try argv.append("../../../zig-out/bin/zar"); - try argv.append(format.llvmFormatToArgument()); - - try argv.append(operation); - try argv.append(zig_ar_archive_name); - try argv.appendSlice(file_names); +const ExpectedOut = struct { + stdout: ?[]const u8 = null, + stderr: ?[]const u8 = null, +}; - if (invoke_zar_as_child_process) { +fn invokeZar(allocator: mem.Allocator, arguments: []const []const u8, test_dir_info: TestDirInfo, expected_out: ExpectedOut) !void { + // argments[0] must be path_to_zar + var invoke_as_child_process = always_invoke_zar_as_child_process; + // At the moment it's easiest to verify the output of stdout/stderr by launching + // zar as a child process, so just doing it like this for now. + invoke_as_child_process = invoke_as_child_process or expected_out.stderr != null; + invoke_as_child_process = invoke_as_child_process or expected_out.stdout != null; + if (invoke_as_child_process) { const result = try std.ChildProcess.exec(.{ .allocator = allocator, - .argv = argv.items, + .argv = arguments, .cwd = test_dir_info.cwd, }); @@ -445,16 +483,41 @@ fn doZarArchiveOperation(comptime format: LlvmFormat, comptime operation: []cons allocator.free(result.stdout); allocator.free(result.stderr); } + + if (expected_out.stdout) |expected_stdout| { + try testing.expectEqualStrings(expected_stdout, result.stdout); + } + if (expected_out.stderr) |expected_stderr| { + try testing.expectEqualStrings(expected_stderr, result.stderr); + } } else { // TODO: don't deinit testing allocator here so that we can confirm // the archiver does everything by the books? var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); - try main.archiveMain(test_dir_info.tmp_dir.dir, arena.allocator(), argv.items); + main.archiveMain(test_dir_info.tmp_dir.dir, arena.allocator(), arguments) catch {}; } } +fn doZarArchiveOperation(comptime format: LlvmFormat, comptime operation: []const u8, file_names: []const []const u8, test_dir_info: TestDirInfo) !void { + const tracy = trace(@src()); + defer tracy.end(); + const allocator = std.testing.allocator; + + var argv = std.ArrayList([]const u8).init(allocator); + defer argv.deinit(); + + try argv.append(path_to_zar); + try argv.append(format.llvmFormatToArgument()); + + try argv.append(operation); + try argv.append(zig_ar_archive_name); + try argv.appendSlice(file_names); + + try invokeZar(allocator, argv.items, test_dir_info, .{}); +} + fn doLlvmArchiveOperation(comptime format: LlvmFormat, comptime operation: []const u8, file_names: []const []const u8, test_dir_info: TestDirInfo) !void { const tracy = trace(@src()); defer tracy.end(); From fe97e7eabec0eb2e9c482bc6d7db5b44136eb50b Mon Sep 17 00:00:00 2001 From: Tom Read Cutting Date: Fri, 19 Aug 2022 19:44:56 +0100 Subject: [PATCH 2/2] Disable new test on Windows --- src/test.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test.zig b/src/test.zig index 9dc97d2..8995b8d 100644 --- a/src/test.zig +++ b/src/test.zig @@ -121,6 +121,9 @@ test "Test Archive Sorted" { } test "Test Argument Errors" { + if (builtin.target.os.tag == .windows) { + return; + } const allocator = std.testing.allocator; var test_dir_info = try TestDirInfo.getInfo(); defer test_dir_info.cleanup();