Skip to content

Commit 7d11c2d

Browse files
committed
{fs,io,posix,debug} tests: clean up and Wasi improvements
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
1 parent a6de11c commit 7d11c2d

File tree

4 files changed

+334
-326
lines changed

4 files changed

+334
-326
lines changed

lib/std/debug.zig

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,12 +1015,12 @@ test printLineFromFileAnyOs {
10151015

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

1022-
// Cases
10231022
{
1023+
// no newlines in file
10241024
const path = try join(allocator, &.{ test_dir_path, "one_line.zig" });
10251025
defer allocator.free(path);
10261026
try test_dir.dir.writeFile(.{ .sub_path = "one_line.zig", .data = "no new lines in this file, but one is printed anyway" });
@@ -1032,6 +1032,7 @@ test printLineFromFileAnyOs {
10321032
output.clearRetainingCapacity();
10331033
}
10341034
{
1035+
// print 1 & 3 of 3-line file
10351036
const path = try join(allocator, &.{ test_dir_path, "three_lines.zig" });
10361037
defer allocator.free(path);
10371038
try test_dir.dir.writeFile(.{
@@ -1052,6 +1053,7 @@ test printLineFromFileAnyOs {
10521053
output.clearRetainingCapacity();
10531054
}
10541055
{
1056+
// mem.page_size boundary crossing line
10551057
const file = try test_dir.dir.createFile("line_overlaps_page_boundary.zig", .{});
10561058
defer file.close();
10571059
const path = try join(allocator, &.{ test_dir_path, "line_overlaps_page_boundary.zig" });
@@ -1068,6 +1070,7 @@ test printLineFromFileAnyOs {
10681070
output.clearRetainingCapacity();
10691071
}
10701072
{
1073+
// ends on mem.page_size boundary
10711074
const file = try test_dir.dir.createFile("file_ends_on_page_boundary.zig", .{});
10721075
defer file.close();
10731076
const path = try join(allocator, &.{ test_dir_path, "file_ends_on_page_boundary.zig" });
@@ -1081,6 +1084,7 @@ test printLineFromFileAnyOs {
10811084
output.clearRetainingCapacity();
10821085
}
10831086
{
1087+
// multi-mem.page_size "line"
10841088
const file = try test_dir.dir.createFile("very_long_first_line_spanning_multiple_pages.zig", .{});
10851089
defer file.close();
10861090
const path = try join(allocator, &.{ test_dir_path, "very_long_first_line_spanning_multiple_pages.zig" });
@@ -1106,6 +1110,7 @@ test printLineFromFileAnyOs {
11061110
output.clearRetainingCapacity();
11071111
}
11081112
{
1113+
// mem.page_size pages of newlines
11091114
const file = try test_dir.dir.createFile("file_of_newlines.zig", .{});
11101115
defer file.close();
11111116
const path = try join(allocator, &.{ test_dir_path, "file_of_newlines.zig" });

lib/std/fs/test.zig

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,8 @@ test "accessAbsolute" {
329329
var tmp = tmpDir(.{});
330330
defer tmp.cleanup();
331331

332-
var arena = ArenaAllocator.init(testing.allocator);
333-
defer arena.deinit();
334-
const allocator = arena.allocator();
335-
336-
const base_path = blk: {
337-
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..] });
338-
break :blk try fs.realpathAlloc(allocator, relative_path);
339-
};
332+
const base_path = try tmp.dir.realpathAlloc(testing.allocator, ".");
333+
defer testing.allocator.free(base_path);
340334

341335
try fs.accessAbsolute(base_path, .{});
342336
}
@@ -347,32 +341,62 @@ test "openDirAbsolute" {
347341
var tmp = tmpDir(.{});
348342
defer tmp.cleanup();
349343

344+
const tmp_ino = (try tmp.dir.stat()).inode;
345+
350346
try tmp.dir.makeDir("subdir");
351-
var arena = ArenaAllocator.init(testing.allocator);
352-
defer arena.deinit();
353-
const allocator = arena.allocator();
347+
const sub_path = try tmp.dir.realpathAlloc(testing.allocator, "subdir");
348+
defer testing.allocator.free(sub_path);
354349

355-
const base_path = blk: {
356-
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..], "subdir" });
357-
break :blk try fs.realpathAlloc(allocator, relative_path);
358-
};
350+
// Can open subdir
351+
var tmp_sub = try fs.openDirAbsolute(sub_path, .{});
352+
defer tmp_sub.close();
353+
354+
const sub_ino = (try tmp_sub.stat()).inode;
355+
356+
{
357+
// Can open parent
358+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".." });
359+
defer testing.allocator.free(dir_path);
360+
361+
var dir = try fs.openDirAbsolute(dir_path, .{});
362+
defer dir.close();
363+
364+
const ino = (try dir.stat()).inode;
365+
try testing.expectEqual(tmp_ino, ino);
366+
}
359367

360368
{
361-
var dir = try fs.openDirAbsolute(base_path, .{});
369+
// Can open subdir + "."
370+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, "." });
371+
defer testing.allocator.free(dir_path);
372+
373+
var dir = try fs.openDirAbsolute(dir_path, .{});
362374
defer dir.close();
375+
376+
const ino = (try dir.stat()).inode;
377+
try testing.expectEqual(sub_ino, ino);
363378
}
364379

365-
for ([_][]const u8{ ".", ".." }) |sub_path| {
366-
const dir_path = try fs.path.join(allocator, &.{ base_path, sub_path });
380+
{
381+
// Can open subdir + "..", with extra "."
382+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".", "..", "." });
383+
defer testing.allocator.free(dir_path);
384+
367385
var dir = try fs.openDirAbsolute(dir_path, .{});
368386
defer dir.close();
387+
388+
const ino = (try dir.stat()).inode;
389+
try testing.expectEqual(tmp_ino, ino);
369390
}
370391
}
371392

372393
test "openDir cwd parent '..'" {
373-
if (native_os == .wasi) return error.SkipZigTest;
374-
375-
var dir = try fs.cwd().openDir("..", .{});
394+
var dir = fs.cwd().openDir("..", .{}) catch |err| {
395+
if (native_os == .wasi and err == error.AccessDenied) {
396+
return; // This is okay. WASI disallows escaping from the fs sandbox
397+
}
398+
return err;
399+
};
376400
defer dir.close();
377401
}
378402

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

391-
var dir = try subdir.openDir("..", .{});
415+
var dir = subdir.openDir("..", .{}) catch |err| {
416+
if (native_os == .wasi and err == error.AccessDenied) {
417+
// This is odd (we're asking for the parent of a subdirectory,
418+
// which should be safely inside the sandbox), but this is the
419+
// current wasmtime behavior (with and without libc).
420+
return error.SkipZigTest;
421+
}
422+
return err;
423+
};
392424
defer dir.close();
393425

394426
if (supports_absolute_paths) {
@@ -421,10 +453,7 @@ test "readLinkAbsolute" {
421453
defer arena.deinit();
422454
const allocator = arena.allocator();
423455

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

429458
{
430459
const target_path = try fs.path.join(allocator, &.{ base_path, "file.txt" });
@@ -770,17 +799,24 @@ test "file operations on directories" {
770799
try testing.expectError(error.IsDir, ctx.dir.createFile(test_dir_name, .{}));
771800
try testing.expectError(error.IsDir, ctx.dir.deleteFile(test_dir_name));
772801
switch (native_os) {
773-
// no error when reading a directory.
774-
.dragonfly, .netbsd => {},
775-
// Currently, WASI will return error.Unexpected (via ENOTCAPABLE) when attempting fd_read on a directory handle.
776-
// TODO: Re-enable on WASI once https://github.com/bytecodealliance/wasmtime/issues/1935 is resolved.
777-
.wasi => {},
802+
.dragonfly, .netbsd => {
803+
// no error when reading a directory. See https://github.com/ziglang/zig/issues/5732
804+
const buf = try ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize));
805+
testing.allocator.free(buf);
806+
},
807+
.wasi => {
808+
// WASI return EBADF, which gets mapped to NotOpenForReading.
809+
// See https://github.com/bytecodealliance/wasmtime/issues/1935
810+
try testing.expectError(error.NotOpenForReading, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
811+
},
778812
else => {
779813
try testing.expectError(error.IsDir, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
780814
},
781815
}
816+
782817
// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
783818
// TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
819+
// Beware, wasmtime v23.0.2 (at least) does not error here
784820
try testing.expectError(error.IsDir, ctx.dir.openFile(test_dir_name, .{ .mode = .read_write }));
785821

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

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

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

13991432
try tmp.dir.makePath("os_test_tmp");
1400-
defer tmp.dir.deleteTree("os_test_tmp") catch {};
14011433

14021434
var dir = try tmp.dir.openDir("os_test_tmp", .{});
14031435
defer dir.close();
@@ -1462,7 +1494,6 @@ test "copyRangeAll" {
14621494
defer tmp.cleanup();
14631495

14641496
try tmp.dir.makePath("os_test_tmp");
1465-
defer tmp.dir.deleteTree("os_test_tmp") catch {};
14661497

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

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

17891817
const subdir_path = try fs.path.join(allocator, &.{ base_path, "./subdir" });
17901818
try fs.makeDirAbsolute(subdir_path);

lib/std/io/test.zig

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ test "File seek ops" {
108108

109109
const tmp_file_name = "temp_test_file.txt";
110110
var file = try tmp.dir.createFile(tmp_file_name, .{});
111-
defer {
112-
file.close();
113-
tmp.dir.deleteFile(tmp_file_name) catch {};
114-
}
111+
defer file.close();
115112

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

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

136133
const tmp_file_name = "temp_test_file.txt";
137134
var file = try tmp.dir.createFile(tmp_file_name, .{});
138-
defer {
139-
file.close();
140-
tmp.dir.deleteFile(tmp_file_name) catch {};
141-
}
135+
defer file.close();
142136

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

162156
const tmp_file_name = "just_a_temporary_file.txt";
163157
var file = try tmp.dir.createFile(tmp_file_name, .{ .read = true });
164-
defer {
165-
file.close();
166-
tmp.dir.deleteFile(tmp_file_name) catch {};
167-
}
158+
defer file.close();
159+
168160
const stat_old = try file.stat();
169161
// Set atime and mtime to 5s before
170162
try file.updateTimes(

0 commit comments

Comments
 (0)