Add tests for confirming bad arguments are handled correctly

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);

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] [-]<operation>[modifiers] [relpos] [count] <archive> [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;
+                },
             }
         }
     }

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,48 @@ test "Test Archive Sorted" {
     try doStandardTests(arena.allocator(), no_dir, &test_sort_names, &test_sort);
 }
 
+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();
+
+    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,42 +463,64 @@ 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,
         });
 
         defer {
             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();