Skip to content

Commit

Permalink
{fs,io,posix,debug} tests: clean up and Wasi improvements
Browse files Browse the repository at this point in the history 
Add `unique_fname` for creating unique-name files when testing in the
default CWD (though tests should still prefer tmpDir()).  Enable the tests
disabled because this is racy (#14968).

Add some WASI-specific behavior handling.

Other cleanups of the tests:
* use `realpathAlloc()`to get fullpath into tmpDir() instances
* use `testing.allocator` where that simplifies things (vs. manual ArenaAllocator for 1 or 2 allocs)
* Trust `TmpDir.cleanup()` to clean up sub-trees
* Remove some unnecessary absolute paths (to enable WASI tests)
* Drop some no-longer necessary `[_][]const u8` explicit types
* Put file cleanups into `defer`

Fixes #14968
  • Loading branch information
@rootbeer
rootbeer committed Aug 8, 2024
1 parent d98770c commit 8efc69e
Show file tree
Hide file tree
Showing 4 changed files with 332 additions and 326 deletions.
9 changes: 7 additions & 2 deletions lib/std/debug.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1013,12 +1013,12 @@ test printLineFromFileAnyOs {

var test_dir = std.testing.tmpDir(.{});
defer test_dir.cleanup();
// Relies on testing.tmpDir internals which is not ideal, but SourceLocation requires paths.
// Relies on testing.tmpDir internals which is not ideal, but SourceLocation requires paths relative to cwd.
const test_dir_path = try join(allocator, &.{ ".zig-cache", "tmp", test_dir.sub_path[0..] });
defer allocator.free(test_dir_path);

// Cases
{
// no newlines in file
const path = try join(allocator, &.{ test_dir_path, "one_line.zig" });
defer allocator.free(path);
try test_dir.dir.writeFile(.{ .sub_path = "one_line.zig", .data = "no new lines in this file, but one is printed anyway" });
Expand All @@ -1030,6 +1030,7 @@ test printLineFromFileAnyOs {
output.clearRetainingCapacity();
}
{
// print 1 & 3 of 3-line file
const path = try join(allocator, &.{ test_dir_path, "three_lines.zig" });
defer allocator.free(path);
try test_dir.dir.writeFile(.{
Expand All @@ -1050,6 +1051,7 @@ test printLineFromFileAnyOs {
output.clearRetainingCapacity();
}
{
// mem.page_size boundary crossing line
const file = try test_dir.dir.createFile("line_overlaps_page_boundary.zig", .{});
defer file.close();
const path = try join(allocator, &.{ test_dir_path, "line_overlaps_page_boundary.zig" });
Expand All @@ -1066,6 +1068,7 @@ test printLineFromFileAnyOs {
output.clearRetainingCapacity();
}
{
// ends on mem.page_size boundary
const file = try test_dir.dir.createFile("file_ends_on_page_boundary.zig", .{});
defer file.close();
const path = try join(allocator, &.{ test_dir_path, "file_ends_on_page_boundary.zig" });
Expand All @@ -1079,6 +1082,7 @@ test printLineFromFileAnyOs {
output.clearRetainingCapacity();
}
{
// multi-mem.page_size "line"
const file = try test_dir.dir.createFile("very_long_first_line_spanning_multiple_pages.zig", .{});
defer file.close();
const path = try join(allocator, &.{ test_dir_path, "very_long_first_line_spanning_multiple_pages.zig" });
Expand All @@ -1104,6 +1108,7 @@ test printLineFromFileAnyOs {
output.clearRetainingCapacity();
}
{
// mem.page_size pages of newlines
const file = try test_dir.dir.createFile("file_of_newlines.zig", .{});
defer file.close();
const path = try join(allocator, &.{ test_dir_path, "file_of_newlines.zig" });
Expand Down
110 changes: 69 additions & 41 deletions lib/std/fs/test.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,8 @@ test "accessAbsolute" {
var tmp = tmpDir(.{});
defer tmp.cleanup();

var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const allocator = arena.allocator();

const base_path = blk: {
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..] });
break :blk try fs.realpathAlloc(allocator, relative_path);
};
const base_path = try tmp.dir.realpathAlloc(testing.allocator, ".");
defer testing.allocator.free(base_path);

try fs.accessAbsolute(base_path, .{});
}
Expand All @@ -347,32 +341,62 @@ test "openDirAbsolute" {
var tmp = tmpDir(.{});
defer tmp.cleanup();

const tmp_ino = (try tmp.dir.stat()).inode;

try tmp.dir.makeDir("subdir");
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const allocator = arena.allocator();
const sub_path = try tmp.dir.realpathAlloc(testing.allocator, "subdir");
defer testing.allocator.free(sub_path);

const base_path = blk: {
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..], "subdir" });
break :blk try fs.realpathAlloc(allocator, relative_path);
};
// Can open subdir
var tmp_sub = try fs.openDirAbsolute(sub_path, .{});
defer tmp_sub.close();

const sub_ino = (try tmp_sub.stat()).inode;

{
// Can open parent
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".." });
defer testing.allocator.free(dir_path);

var dir = try fs.openDirAbsolute(dir_path, .{});
defer dir.close();

const ino = (try dir.stat()).inode;
try testing.expectEqual(tmp_ino, ino);
}

{
var dir = try fs.openDirAbsolute(base_path, .{});
// Can open subdir + "."
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, "." });
defer testing.allocator.free(dir_path);

var dir = try fs.openDirAbsolute(dir_path, .{});
defer dir.close();

const ino = (try dir.stat()).inode;
try testing.expectEqual(sub_ino, ino);
}

for ([_][]const u8{ ".", ".." }) |sub_path| {
const dir_path = try fs.path.join(allocator, &.{ base_path, sub_path });
{
// Can open subdir + "..", with extra "."
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".", "..", "." });
defer testing.allocator.free(dir_path);

var dir = try fs.openDirAbsolute(dir_path, .{});
defer dir.close();

const ino = (try dir.stat()).inode;
try testing.expectEqual(tmp_ino, ino);
}
}

test "openDir cwd parent '..'" {
if (native_os == .wasi) return error.SkipZigTest;

var dir = try fs.cwd().openDir("..", .{});
var dir = fs.cwd().openDir("..", .{}) catch |err| {
if (native_os == .wasi and err == error.AccessDenied) {
return; // This is okay. WASI disallows escaping from the fs sandbox
}
return err;
};
defer dir.close();
}

Expand All @@ -388,7 +412,15 @@ test "openDir non-cwd parent '..'" {
var subdir = try tmp.dir.makeOpenPath("subdir", .{});
defer subdir.close();

var dir = try subdir.openDir("..", .{});
var dir = subdir.openDir("..", .{}) catch |err| {
if (native_os == .wasi and err == error.AccessDenied) {
// This is odd (we're asking for the parent of a subdirectory,
// which should be safely inside the sandbox), but this is the
// current wasmtime behavior (with and without libc).
return error.SkipZigTest;
}
return err;
};
defer dir.close();

if (supports_absolute_paths) {
Expand Down Expand Up @@ -421,10 +453,7 @@ test "readLinkAbsolute" {
defer arena.deinit();
const allocator = arena.allocator();

const base_path = blk: {
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..] });
break :blk try fs.realpathAlloc(allocator, relative_path);
};
const base_path = try tmp.dir.realpathAlloc(allocator, ".");

{
const target_path = try fs.path.join(allocator, &.{ base_path, "file.txt" });
Expand Down Expand Up @@ -770,17 +799,24 @@ test "file operations on directories" {
try testing.expectError(error.IsDir, ctx.dir.createFile(test_dir_name, .{}));
try testing.expectError(error.IsDir, ctx.dir.deleteFile(test_dir_name));
switch (native_os) {
// no error when reading a directory.
.dragonfly, .netbsd => {},
// Currently, WASI will return error.Unexpected (via ENOTCAPABLE) when attempting fd_read on a directory handle.
// TODO: Re-enable on WASI once https://github.com/bytecodealliance/wasmtime/issues/1935 is resolved.
.wasi => {},
.dragonfly, .netbsd => {
// no error when reading a directory. See https://github.com/ziglang/zig/issues/5732
const buf = try ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize));
testing.allocator.free(buf);
},
.wasi => {
// WASI return EBADF, which gets mapped to NotOpenForReading.
// See https://github.com/bytecodealliance/wasmtime/issues/1935
try testing.expectError(error.NotOpenForReading, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
},
else => {
try testing.expectError(error.IsDir, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
},
}

// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
// TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
// Beware, wasmtime v23.0.2 (at least) does not error here
try testing.expectError(error.IsDir, ctx.dir.openFile(test_dir_name, .{ .mode = .read_write }));

if (ctx.path_type == .absolute and supports_absolute_paths) {
Expand Down Expand Up @@ -1004,10 +1040,7 @@ test "renameAbsolute" {
defer arena.deinit();
const allocator = arena.allocator();

const base_path = blk: {
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp_dir.sub_path[0..] });
break :blk try fs.realpathAlloc(allocator, relative_path);
};
const base_path = try tmp_dir.dir.realpathAlloc(allocator, ".");

try testing.expectError(error.FileNotFound, fs.renameAbsolute(
try fs.path.join(allocator, &.{ base_path, "missing_file_name" }),
Expand Down Expand Up @@ -1397,7 +1430,6 @@ test "sendfile" {
defer tmp.cleanup();

try tmp.dir.makePath("os_test_tmp");
defer tmp.dir.deleteTree("os_test_tmp") catch {};

var dir = try tmp.dir.openDir("os_test_tmp", .{});
defer dir.close();
Expand Down Expand Up @@ -1462,7 +1494,6 @@ test "copyRangeAll" {
defer tmp.cleanup();

try tmp.dir.makePath("os_test_tmp");
defer tmp.dir.deleteTree("os_test_tmp") catch {};

var dir = try tmp.dir.openDir("os_test_tmp", .{});
defer dir.close();
Expand Down Expand Up @@ -1781,10 +1812,7 @@ test "'.' and '..' in absolute functions" {
defer arena.deinit();
const allocator = arena.allocator();

const base_path = blk: {
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..] });
break :blk try fs.realpathAlloc(allocator, relative_path);
};
const base_path = try tmp.dir.realpathAlloc(allocator, ".");

const subdir_path = try fs.path.join(allocator, &.{ base_path, "./subdir" });
try fs.makeDirAbsolute(subdir_path);
Expand Down
16 changes: 4 additions & 12 deletions lib/std/io/test.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,7 @@ test "File seek ops" {

const tmp_file_name = "temp_test_file.txt";
var file = try tmp.dir.createFile(tmp_file_name, .{});
defer {
file.close();
tmp.dir.deleteFile(tmp_file_name) catch {};
}
defer file.close();

try file.writeAll(&([_]u8{0x55} ** 8192));

Expand All @@ -135,10 +132,7 @@ test "setEndPos" {

const tmp_file_name = "temp_test_file.txt";
var file = try tmp.dir.createFile(tmp_file_name, .{});
defer {
file.close();
tmp.dir.deleteFile(tmp_file_name) catch {};
}
defer file.close();

// Verify that the file size changes and the file offset is not moved
try std.testing.expect((try file.getEndPos()) == 0);
Expand All @@ -161,10 +155,8 @@ test "updateTimes" {

const tmp_file_name = "just_a_temporary_file.txt";
var file = try tmp.dir.createFile(tmp_file_name, .{ .read = true });
defer {
file.close();
tmp.dir.deleteFile(tmp_file_name) catch {};
}
defer file.close();

const stat_old = try file.stat();
// Set atime and mtime to 5s before
try file.updateTimes(
Expand Down
Loading

0 comments on commit 8efc69e

Please sign in to comment.