Commit d0e288ab18

Pat Tullmann <pat.github@tullmann.org>
2024-09-17 06:52:28
lib/std/posix/test.zig: enable disabled tests using CWD
Four tests in lib/std/posix/test.zig were disabled because they created fixed-name files in the current working directory, and this caused problems if tests were running in parallel with other build's tests. This PR fixes those tests to all use `std.testing.tmpDir` to create unique temporary names and directories. Also clean the tests up to more consistently use `defer` to clean up, or to just rely on tmpDir cleanup to remove individual files. Working on these tests revealed a bunch of stale WASI code paths in posix.zig, fixed by replacing stale `wast.AT.FDCWD` references with just `AT.FDCWD`. Fixes #14968.
1 parent 8d824df
Changed files (2)
lib
lib/std/posix/test.zig
@@ -44,13 +44,12 @@ test "check WASI CWD" {
     }
 }
 
-test "chdir smoke test" {
+test "chdir absolute parent" {
     if (native_os == .wasi) return error.SkipZigTest;
 
-    if (true) {
-        // https://github.com/ziglang/zig/issues/14968
-        return error.SkipZigTest;
-    }
+    // Restore default CWD at end of test.
+    const orig_cwd = try fs.cwd().openDir(".", .{});
+    defer orig_cwd.setAsCwd() catch unreachable;
 
     // Get current working directory path
     var old_cwd_buf: [fs.max_path_bytes]u8 = undefined;
@@ -65,47 +64,46 @@ test "chdir smoke test" {
     }
 
     // Next, change current working directory to one level above
-    if (native_os != .wasi) { // WASI does not support navigating outside of Preopens
-        const parent = fs.path.dirname(old_cwd) orelse unreachable; // old_cwd should be absolute
-        try posix.chdir(parent);
+    const parent = fs.path.dirname(old_cwd) orelse unreachable; // old_cwd should be absolute
+    try posix.chdir(parent);
 
-        // Restore cwd because process may have other tests that do not tolerate chdir.
-        defer posix.chdir(old_cwd) catch unreachable;
+    var new_cwd_buf: [fs.max_path_bytes]u8 = undefined;
+    const new_cwd = try posix.getcwd(new_cwd_buf[0..]);
+    try expect(mem.eql(u8, parent, new_cwd));
+}
 
-        var new_cwd_buf: [fs.max_path_bytes]u8 = undefined;
-        const new_cwd = try posix.getcwd(new_cwd_buf[0..]);
-        try expect(mem.eql(u8, parent, new_cwd));
-    }
+test "chdir relative" {
+    if (native_os == .wasi) return error.SkipZigTest;
 
-    // Next, change current working directory to a temp directory one level below
-    {
-        // Create a tmp directory
-        var tmp_dir_buf: [fs.max_path_bytes]u8 = undefined;
-        const tmp_dir_path = path: {
-            var allocator = std.heap.FixedBufferAllocator.init(&tmp_dir_buf);
-            break :path try fs.path.resolve(allocator.allocator(), &[_][]const u8{ old_cwd, "zig-test-tmp" });
-        };
-        var tmp_dir = try fs.cwd().makeOpenPath("zig-test-tmp", .{});
+    var tmp = tmpDir(.{});
+    defer tmp.cleanup();
 
-        // Change current working directory to tmp directory
-        try posix.chdir("zig-test-tmp");
+    // Restore default CWD at end of test.
+    const orig_cwd = try fs.cwd().openDir(".", .{});
+    defer orig_cwd.setAsCwd() catch unreachable;
 
-        var new_cwd_buf: [fs.max_path_bytes]u8 = undefined;
-        const new_cwd = try posix.getcwd(new_cwd_buf[0..]);
+    // Use the tmpDir parent_dir as the "base" for the test. Then cd into the child
+    try tmp.parent_dir.setAsCwd();
 
-        // On Windows, fs.path.resolve returns an uppercase drive letter, but the drive letter returned by getcwd may be lowercase
-        var resolved_cwd_buf: [fs.max_path_bytes]u8 = undefined;
-        const resolved_cwd = path: {
-            var allocator = std.heap.FixedBufferAllocator.init(&resolved_cwd_buf);
-            break :path try fs.path.resolve(allocator.allocator(), &[_][]const u8{new_cwd});
-        };
-        try expect(mem.eql(u8, tmp_dir_path, resolved_cwd));
+    // Capture base working directory path, to build expected full path
+    var base_cwd_buf: [fs.max_path_bytes]u8 = undefined;
+    const base_cwd = try posix.getcwd(base_cwd_buf[0..]);
 
-        // Restore cwd because process may have other tests that do not tolerate chdir.
-        tmp_dir.close();
-        posix.chdir(old_cwd) catch unreachable;
-        try fs.cwd().deleteDir("zig-test-tmp");
-    }
+    const dir_name = &tmp.sub_path;
+    const expected_path = try fs.path.resolve(a, &.{ base_cwd, dir_name });
+    defer a.free(expected_path);
+
+    // change current working directory to new directory
+    try posix.chdir(dir_name);
+
+    var new_cwd_buf: [fs.max_path_bytes]u8 = undefined;
+    const new_cwd = try posix.getcwd(new_cwd_buf[0..]);
+
+    // On Windows, fs.path.resolve returns an uppercase drive letter, but the drive letter returned by getcwd may be lowercase
+    const resolved_cwd = try fs.path.resolve(a, &.{new_cwd});
+    defer a.free(resolved_cwd);
+
+    try expect(mem.eql(u8, expected_path, resolved_cwd));
 }
 
 test "open smoke test" {
@@ -222,44 +220,42 @@ test "openat smoke test" {
 }
 
 test "symlink with relative paths" {
-    if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
+    if (native_os == .wasi) return error.SkipZigTest; // Can symlink, but can't change into tmpDir
 
-    if (true) {
-        // https://github.com/ziglang/zig/issues/14968
-        return error.SkipZigTest;
-    }
-    const cwd = fs.cwd();
-    cwd.deleteFile("file.txt") catch {};
-    cwd.deleteFile("symlinked") catch {};
+    var tmp = tmpDir(.{});
+    defer tmp.cleanup();
+
+    const target_name = "symlink-target";
+    const symlink_name = "symlinker";
 
-    // First, try relative paths in cwd
-    try cwd.writeFile(.{ .sub_path = "file.txt", .data = "nonsense" });
+    // Restore default CWD at end of test.
+    const orig_cwd = try fs.cwd().openDir(".", .{});
+    defer orig_cwd.setAsCwd() catch unreachable;
+
+    // Create the target file
+    try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "nonsense" });
+
+    // Want to test relative paths, so cd into the tmpdir for this test
+    try tmp.dir.setAsCwd();
 
     if (native_os == .windows) {
-        std.os.windows.CreateSymbolicLink(
-            cwd.fd,
-            &[_]u16{ 's', 'y', 'm', 'l', 'i', 'n', 'k', 'e', 'd' },
-            &[_:0]u16{ 'f', 'i', 'l', 'e', '.', 't', 'x', 't' },
-            false,
-        ) catch |err| switch (err) {
+        const wtarget_name = try std.unicode.wtf8ToWtf16LeAllocZ(a, target_name);
+        const wsymlink_name = try std.unicode.wtf8ToWtf16LeAllocZ(a, symlink_name);
+        defer a.free(wtarget_name);
+        defer a.free(wsymlink_name);
+
+        std.os.windows.CreateSymbolicLink(tmp.dir.fd, wsymlink_name, wtarget_name, false) catch |err| switch (err) {
             // Symlink requires admin privileges on windows, so this test can legitimately fail.
-            error.AccessDenied => {
-                try cwd.deleteFile("file.txt");
-                try cwd.deleteFile("symlinked");
-                return error.SkipZigTest;
-            },
+            error.AccessDenied => return error.SkipZigTest,
             else => return err,
         };
     } else {
-        try posix.symlink("file.txt", "symlinked");
+        try posix.symlink(target_name, symlink_name);
     }
 
     var buffer: [fs.max_path_bytes]u8 = undefined;
-    const given = try posix.readlink("symlinked", buffer[0..]);
-    try expect(mem.eql(u8, "file.txt", given));
-
-    try cwd.deleteFile("file.txt");
-    try cwd.deleteFile("symlinked");
+    const given = try posix.readlink(symlink_name, buffer[0..]);
+    try expect(mem.eql(u8, target_name, given));
 }
 
 test "readlink on Windows" {
@@ -277,90 +273,95 @@ fn testReadlink(target_path: []const u8, symlink_path: []const u8) !void {
 }
 
 test "link with relative paths" {
-    if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
+    if (native_os == .wasi) return error.SkipZigTest; // Can link, but can't change into tmpDir
+    if (builtin.cpu.arch == .riscv32 and builtin.os.tag == .linux and !builtin.link_libc) return error.SkipZigTest; // No `fstat()`.
 
     switch (native_os) {
         .wasi, .linux, .solaris, .illumos => {},
         else => return error.SkipZigTest,
     }
-    if (true) {
-        // https://github.com/ziglang/zig/issues/14968
-        return error.SkipZigTest;
-    }
-    var cwd = fs.cwd();
 
-    cwd.deleteFile("example.txt") catch {};
-    cwd.deleteFile("new.txt") catch {};
+    var tmp = tmpDir(.{});
+    defer tmp.cleanup();
+
+    // Restore default CWD at end of test.
+    const orig_cwd = try fs.cwd().openDir(".", .{});
+    defer orig_cwd.setAsCwd() catch unreachable;
+
+    const target_name = "link-target";
+    const link_name = "newlink";
+
+    try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "example" });
 
-    try cwd.writeFile(.{ .sub_path = "example.txt", .data = "example" });
-    try posix.link("example.txt", "new.txt");
+    // Test 1: create the relative link from inside tmp
+    try tmp.dir.setAsCwd();
+    try posix.link(target_name, link_name);
 
-    const efd = try cwd.openFile("example.txt", .{});
+    // Verify
+    const efd = try tmp.dir.openFile(target_name, .{});
     defer efd.close();
 
-    const nfd = try cwd.openFile("new.txt", .{});
+    const nfd = try tmp.dir.openFile(link_name, .{});
     defer nfd.close();
 
     {
         const estat = try posix.fstat(efd.handle);
         const nstat = try posix.fstat(nfd.handle);
-
         try testing.expectEqual(estat.ino, nstat.ino);
         try testing.expectEqual(@as(@TypeOf(nstat.nlink), 2), nstat.nlink);
     }
 
-    try posix.unlink("new.txt");
+    // Test 2: Remove the link and see the stats update
+    try posix.unlink(link_name);
 
     {
         const estat = try posix.fstat(efd.handle);
         try testing.expectEqual(@as(@TypeOf(estat.nlink), 1), estat.nlink);
     }
-
-    try cwd.deleteFile("example.txt");
 }
 
 test "linkat with different directories" {
-    if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
+    if (builtin.cpu.arch == .riscv32 and builtin.os.tag == .linux and !builtin.link_libc) return error.SkipZigTest; // No `fstatat()`.
 
     switch (native_os) {
         .wasi, .linux, .solaris, .illumos => {},
         else => return error.SkipZigTest,
     }
-    if (true) {
-        // https://github.com/ziglang/zig/issues/14968
-        return error.SkipZigTest;
-    }
-    var cwd = fs.cwd();
+
     var tmp = tmpDir(.{});
+    defer tmp.cleanup();
+
+    const target_name = "link-target";
+    const link_name = "newlink";
 
-    cwd.deleteFile("example.txt") catch {};
-    tmp.dir.deleteFile("new.txt") catch {};
+    const subdir = try tmp.dir.makeOpenPath("subdir", .{});
 
-    try cwd.writeFile(.{ .sub_path = "example.txt", .data = "example" });
-    try posix.linkat(cwd.fd, "example.txt", tmp.dir.fd, "new.txt", 0);
+    defer tmp.dir.deleteFile(target_name) catch {};
+    try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "example" });
 
-    const efd = try cwd.openFile("example.txt", .{});
+    // Test 1: link from file in subdir back up to target in parent directory
+    try posix.linkat(tmp.dir.fd, target_name, subdir.fd, link_name, 0);
+
+    const efd = try tmp.dir.openFile(target_name, .{});
     defer efd.close();
 
-    const nfd = try tmp.dir.openFile("new.txt", .{});
+    const nfd = try subdir.openFile(link_name, .{});
+    defer nfd.close();
 
     {
-        defer nfd.close();
         const estat = try posix.fstat(efd.handle);
         const nstat = try posix.fstat(nfd.handle);
-
         try testing.expectEqual(estat.ino, nstat.ino);
         try testing.expectEqual(@as(@TypeOf(nstat.nlink), 2), nstat.nlink);
     }
 
-    try posix.unlinkat(tmp.dir.fd, "new.txt", 0);
+    // Test 2: remove link
+    try posix.unlinkat(subdir.fd, link_name, 0);
 
     {
         const estat = try posix.fstat(efd.handle);
         try testing.expectEqual(@as(@TypeOf(estat.nlink), 1), estat.nlink);
     }
-
-    try cwd.deleteFile("example.txt");
 }
 
 test "fstatat" {
@@ -979,7 +980,7 @@ test "POSIX file locking with fcntl" {
         return error.SkipZigTest;
     }
 
-    var tmp = std.testing.tmpDir(.{});
+    var tmp = tmpDir(.{});
     defer tmp.cleanup();
 
     // Create a temporary lock file
lib/std/posix.zig
@@ -2104,7 +2104,7 @@ pub fn symlink(target_path: []const u8, sym_link_path: []const u8) SymLinkError!
     if (native_os == .windows) {
         @compileError("symlink is not supported on Windows; use std.os.windows.CreateSymbolicLink instead");
     } else if (native_os == .wasi and !builtin.link_libc) {
-        return symlinkat(target_path, wasi.AT.FDCWD, sym_link_path);
+        return symlinkat(target_path, AT.FDCWD, sym_link_path);
     }
     const target_path_c = try toPosixPath(target_path);
     const sym_link_path_c = try toPosixPath(sym_link_path);
@@ -2274,7 +2274,7 @@ pub fn linkZ(oldpath: [*:0]const u8, newpath: [*:0]const u8) LinkError!void {
 /// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
 pub fn link(oldpath: []const u8, newpath: []const u8) LinkError!void {
     if (native_os == .wasi and !builtin.link_libc) {
-        return linkat(wasi.AT.FDCWD, oldpath, wasi.AT.FDCWD, newpath, 0) catch |err| switch (err) {
+        return linkat(AT.FDCWD, oldpath, AT.FDCWD, newpath, 0) catch |err| switch (err) {
             error.NotDir => unreachable, // link() does not support directories
             else => |e| return e,
         };
@@ -2411,7 +2411,7 @@ pub const UnlinkError = error{
 /// See also `unlinkZ`.
 pub fn unlink(file_path: []const u8) UnlinkError!void {
     if (native_os == .wasi and !builtin.link_libc) {
-        return unlinkat(wasi.AT.FDCWD, file_path, 0) catch |err| switch (err) {
+        return unlinkat(AT.FDCWD, file_path, 0) catch |err| switch (err) {
             error.DirNotEmpty => unreachable, // only occurs when targeting directories
             else => |e| return e,
         };
@@ -2605,7 +2605,7 @@ pub const RenameError = error{
 /// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
 pub fn rename(old_path: []const u8, new_path: []const u8) RenameError!void {
     if (native_os == .wasi and !builtin.link_libc) {
-        return renameat(wasi.AT.FDCWD, old_path, wasi.AT.FDCWD, new_path);
+        return renameat(AT.FDCWD, old_path, AT.FDCWD, new_path);
     } else if (native_os == .windows) {
         const old_path_w = try windows.sliceToPrefixedFileW(null, old_path);
         const new_path_w = try windows.sliceToPrefixedFileW(null, new_path);
@@ -3000,7 +3000,7 @@ pub const MakeDirError = error{
 /// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding.
 pub fn mkdir(dir_path: []const u8, mode: mode_t) MakeDirError!void {
     if (native_os == .wasi and !builtin.link_libc) {
-        return mkdirat(wasi.AT.FDCWD, dir_path, mode);
+        return mkdirat(AT.FDCWD, dir_path, mode);
     } else if (native_os == .windows) {
         const dir_path_w = try windows.sliceToPrefixedFileW(null, dir_path);
         return mkdirW(dir_path_w.span(), mode);
@@ -3089,7 +3089,7 @@ pub const DeleteDirError = error{
 /// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding.
 pub fn rmdir(dir_path: []const u8) DeleteDirError!void {
     if (native_os == .wasi and !builtin.link_libc) {
-        return unlinkat(wasi.AT.FDCWD, dir_path, AT.REMOVEDIR) catch |err| switch (err) {
+        return unlinkat(AT.FDCWD, dir_path, AT.REMOVEDIR) catch |err| switch (err) {
             error.FileSystem => unreachable, // only occurs when targeting files
             error.IsDir => unreachable, // only occurs when targeting files
             else => |e| return e,
@@ -3278,7 +3278,7 @@ pub const ReadLinkError = error{
 /// On other platforms, the result is an opaque sequence of bytes with no particular encoding.
 pub fn readlink(file_path: []const u8, out_buffer: []u8) ReadLinkError![]u8 {
     if (native_os == .wasi and !builtin.link_libc) {
-        return readlinkat(wasi.AT.FDCWD, file_path, out_buffer);
+        return readlinkat(AT.FDCWD, file_path, out_buffer);
     } else if (native_os == .windows) {
         const file_path_w = try windows.sliceToPrefixedFileW(null, file_path);
         return readlinkW(file_path_w.span(), out_buffer);
@@ -4893,7 +4893,7 @@ pub fn access(path: []const u8, mode: u32) AccessError!void {
         _ = try windows.GetFileAttributesW(path_w.span().ptr);
         return;
     } else if (native_os == .wasi and !builtin.link_libc) {
-        return faccessat(wasi.AT.FDCWD, path, mode, 0);
+        return faccessat(AT.FDCWD, path, mode, 0);
     }
     const path_c = try toPosixPath(path);
     return accessZ(&path_c, mode);