From 95a0e127b3f9528d25e0aefefc238d082a57bdc3 Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Sat, 6 Jan 2024 13:22:14 -0800 Subject: [PATCH 1/4] std/fs/test.zig: quote . and .. in test names The test runner uses "." in its output between the test module and the test name, so quote the leading '.' in these test names to make them easier to read. --- lib/std/fs/test.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 4a8cc19517c5..abdfee284fab 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -317,14 +317,14 @@ test "openDirAbsolute" { } } -test "openDir cwd parent .." { +test "openDir cwd parent '..'" { if (builtin.os.tag == .wasi) return error.SkipZigTest; var dir = try fs.cwd().openDir("..", .{}); defer dir.close(); } -test "openDir non-cwd parent .." { +test "openDir non-cwd parent '..'" { switch (builtin.os.tag) { .wasi, .netbsd, .openbsd => return error.SkipZigTest, else => {}, @@ -1674,7 +1674,7 @@ test "walker without fully iterating" { try testing.expectEqual(@as(usize, 1), num_walked); } -test ". and .. in fs.Dir functions" { +test "'.' and '..' in fs.Dir functions" { if (builtin.os.tag == .wasi and builtin.link_libc) return error.SkipZigTest; if (builtin.os.tag == .windows and builtin.cpu.arch == .aarch64) { @@ -1714,7 +1714,7 @@ test ". and .. in fs.Dir functions" { }.impl); } -test ". and .. in absolute functions" { +test "'.' and '..' in absolute functions" { if (builtin.os.tag == .wasi) return error.SkipZigTest; var tmp = tmpDir(.{}); From c36962bb197abf66e2eb94068dc2a53b098fff9d Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Mon, 23 Oct 2023 15:00:17 -0700 Subject: [PATCH 2/4] std/fs/test.zig: Factor out the symlink-creation wrappers Because creation of a symlink can fail on Windows with an Access Denied error (https://learn.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links) any tests that need a symbolic link "skip" if they run into this problem. This change factors out a "setupSymbolicLink()" routine to make this clearer, a bit tighter, and easier to use in future tests. I also collapsed the "symlink in parent directory" test into the existing "Dir.readlink" test, because the latter uses the more comprehensive testWithAllSupportedPathTypes wrapper. --- lib/std/fs/test.zig | 123 ++++++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 74 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index abdfee284fab..95f32ed6d4ac 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -10,6 +10,7 @@ const ArenaAllocator = std.heap.ArenaAllocator; const Dir = std.fs.Dir; const File = std.fs.File; const tmpDir = testing.tmpDir; +const SymLinkFlags = std.fs.Dir.SymLinkFlags; const PathType = enum { relative, @@ -119,6 +120,25 @@ fn testWithAllSupportedPathTypes(test_func: anytype) !void { } } +// For use in test setup. If the symlink creation fails on Windows with +// AccessDenied, then make the test failure silent (it is not a Zig failure). +fn setupSymlink(dir: Dir, target: []const u8, link: []const u8, flags: SymLinkFlags) !void { + return dir.symLink(target, link, flags) catch |err| switch (err) { + // Symlink requires admin privileges on windows, so this test can legitimately fail. + error.AccessDenied => if (builtin.os.tag == .windows) return error.SkipZigTest else return err, + else => return err, + }; +} + +// For use in test setup. If the symlink creation fails on Windows with +// AccessDenied, then make the test failure silent (it is not a Zig failure). +fn setupSymlinkAbsolute(target: []const u8, link: []const u8, flags: SymLinkFlags) !void { + return fs.symLinkAbsolute(target, link, flags) catch |err| switch (err) { + error.AccessDenied => if (builtin.os.tag == .windows) return error.SkipZigTest else return err, + else => return err, + }; +} + test "Dir.readLink" { try testWithAllSupportedPathTypes(struct { fn impl(ctx: *TestContext) !void { @@ -128,31 +148,33 @@ test "Dir.readLink" { const dir_target_path = try ctx.transformPath("subdir"); try ctx.dir.makeDir(dir_target_path); - { - // Create symbolic link by path - ctx.dir.symLink(file_target_path, "symlink1", .{}) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; - try testReadLink(ctx.dir, file_target_path, "symlink1"); - } - { - // Create symbolic link by path - ctx.dir.symLink(dir_target_path, "symlink2", .{ .is_directory = true }) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; - try testReadLink(ctx.dir, dir_target_path, "symlink2"); - } + // test 1: symlink to a file + try setupSymlink(ctx.dir, file_target_path, "symlink1", .{}); + try testReadLink(ctx.dir, file_target_path, "symlink1"); + + // test 2: symlink to a directory (can be different on Windows) + try setupSymlink(ctx.dir, dir_target_path, "symlink2", .{ .is_directory = true }); + try testReadLink(ctx.dir, dir_target_path, "symlink2"); + + // test 3: relative path symlink + const parent_file = ".." ++ fs.path.sep_str ++ "target.txt"; + var subdir = try ctx.dir.makeOpenPath("subdir", .{}); + defer subdir.close(); + try setupSymlink(subdir, parent_file, "relative-link.txt", .{}); + try testReadLink(subdir, parent_file, "relative-link.txt"); } }.impl); } fn testReadLink(dir: Dir, target_path: []const u8, symlink_path: []const u8) !void { var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; - const given = try dir.readLink(symlink_path, buffer[0..]); + const actual = try dir.readLink(symlink_path, buffer[0..]); + try testing.expectEqualStrings(target_path, actual); +} + +fn testReadLinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void { + var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; + const given = try fs.readLinkAbsolute(symlink_path, buffer[0..]); try testing.expectEqualStrings(target_path, given); } @@ -169,11 +191,7 @@ test "File.stat on a File that is a symlink returns Kind.sym_link" { const dir_target_path = try ctx.transformPath("subdir"); try ctx.dir.makeDir(dir_target_path); - ctx.dir.symLink(dir_target_path, "symlink", .{ .is_directory = true }) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + try setupSymlink(ctx.dir, dir_target_path, "symlink", .{ .is_directory = true }); var symlink = switch (builtin.target.os.tag) { .windows => windows_symlink: { @@ -238,23 +256,6 @@ test "File.stat on a File that is a symlink returns Kind.sym_link" { }.impl); } -test "relative symlink to parent directory" { - var tmp = tmpDir(.{}); - defer tmp.cleanup(); - - var subdir = try tmp.dir.makeOpenPath("subdir", .{}); - defer subdir.close(); - - const expected_link_name = ".." ++ std.fs.path.sep_str ++ "b.txt"; - - try subdir.symLink(expected_link_name, "a.txt", .{}); - - var buf: [1000]u8 = undefined; - const link_name = try subdir.readLink("a.txt", &buf); - - try testing.expectEqualStrings(expected_link_name, link_name); -} - test "openDir" { try testWithAllSupportedPathTypes(struct { fn impl(ctx: *TestContext) !void { @@ -373,33 +374,19 @@ test "readLinkAbsolute" { const symlink_path = try fs.path.join(allocator, &.{ base_path, "symlink1" }); // Create symbolic link by path - fs.symLinkAbsolute(target_path, symlink_path, .{}) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + try setupSymlinkAbsolute(target_path, symlink_path, .{}); try testReadLinkAbsolute(target_path, symlink_path); } { const target_path = try fs.path.join(allocator, &.{ base_path, "subdir" }); const symlink_path = try fs.path.join(allocator, &.{ base_path, "symlink2" }); - // Create symbolic link by path - fs.symLinkAbsolute(target_path, symlink_path, .{ .is_directory = true }) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + // Create symbolic link to a directory by path + try setupSymlinkAbsolute(target_path, symlink_path, .{ .is_directory = true }); try testReadLinkAbsolute(target_path, symlink_path); } } -fn testReadLinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void { - var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; - const given = try fs.readLinkAbsolute(symlink_path, buffer[0..]); - try testing.expectEqualStrings(target_path, given); -} - test "Dir.Iterator" { var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); @@ -1005,11 +992,7 @@ test "deleteTree does not follow symlinks" { var a = try tmp.dir.makeOpenPath("a", .{}); defer a.close(); - a.symLink("../b", "b", .{ .is_directory = true }) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + try setupSymlink(a, "../b", "b", .{ .is_directory = true }); } try tmp.dir.deleteTree("a"); @@ -1024,11 +1007,7 @@ test "deleteTree on a symlink" { // Symlink to a file try tmp.dir.writeFile("file", ""); - tmp.dir.symLink("file", "filelink", .{}) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + try setupSymlink(tmp.dir, "file", "filelink", .{}); try tmp.dir.deleteTree("filelink"); try testing.expectError(error.FileNotFound, tmp.dir.access("filelink", .{})); @@ -1036,11 +1015,7 @@ test "deleteTree on a symlink" { // Symlink to a directory try tmp.dir.makePath("dir"); - tmp.dir.symLink("dir", "dirlink", .{ .is_directory = true }) catch |err| switch (err) { - // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => return error.SkipZigTest, - else => return err, - }; + try setupSymlink(tmp.dir, "dir", "dirlink", .{ .is_directory = true }); try tmp.dir.deleteTree("dirlink"); try testing.expectError(error.FileNotFound, tmp.dir.access("dirlink", .{})); @@ -1123,7 +1098,7 @@ test "makepath through existing valid symlink" { defer tmp.cleanup(); try tmp.dir.makeDir("realfolder"); - try tmp.dir.symLink("." ++ fs.path.sep_str ++ "realfolder", "working-symlink", .{}); + try setupSymlink(tmp.dir, "." ++ fs.path.sep_str ++ "realfolder", "working-symlink", .{}); try tmp.dir.makePath("working-symlink" ++ fs.path.sep_str ++ "in-realfolder"); From d35bdc8ee7e414caec619488691a22e4d79aae14 Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Sat, 6 Jan 2024 13:27:59 -0800 Subject: [PATCH 3/4] std/fs/test.zig: Try harder to clean up locking files --- lib/std/fs/test.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 95f32ed6d4ac..edcdf40ef271 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -1559,11 +1559,11 @@ test "open file with exclusive nonblocking lock twice (absolute paths)" { const filename = try fs.path.resolve(gpa, &.{ cwd, sub_path }); defer gpa.free(filename); + defer fs.deleteFileAbsolute(filename) catch {}; // createFileAbsolute can leave files on failures const file1 = try fs.createFileAbsolute(filename, .{ .lock = .exclusive, .lock_nonblocking = true, }); - defer fs.deleteFileAbsolute(filename) catch {}; const file2 = fs.createFileAbsolute(filename, .{ .lock = .exclusive, From ff5613873f4492e25c5779183284a29e7be2f551 Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Thu, 11 Jan 2024 21:03:36 -0800 Subject: [PATCH 4/4] std/fs/test.zig: Add statFile() tests of dangling symlink Create a dangling symlink and check that statFile works with it. --- lib/std/fs/test.zig | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index edcdf40ef271..28d1d111d9d2 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -661,6 +661,19 @@ test "Dir.statFile" { }.impl); } +test "statFile on dangling symlink" { + try testWithAllSupportedPathTypes(struct { + fn impl(ctx: *TestContext) !void { + const symlink_name = try ctx.transformPath("dangling-symlink"); + const symlink_target = "." ++ fs.path.sep_str ++ "doesnotexist"; + + try setupSymlink(ctx.dir, symlink_target, symlink_name, .{}); + + try std.testing.expectError(error.FileNotFound, ctx.dir.statFile(symlink_name)); + } + }.impl); +} + test "directory operations on files" { try testWithAllSupportedPathTypes(struct { fn impl(ctx: *TestContext) !void {