Skip to content

Commit eace31c

Browse files
rootbeeralexrp
authored andcommitted
std/lib: {fs,io,posix} test clean up
* use `tmp.dir.realpathAlloc()` to get full path into tmpDir instances * use `testing.allocator` where that simplifies things (vs. manual ArenaAllocator for 1 or 2 allocs) * Trust `TmpDir.cleanup()` to clean up contained files and sub-trees * Remove some unnecessary absolute paths (enabling WASI to run the tests) * Drop some no-longer necessary `[_][]const u8` casts * Add scopes to reduce `var` usage in favor of `const`
1 parent 02f63fd commit eace31c

File tree

3 files changed

+223
-239
lines changed

3 files changed

+223
-239
lines changed

lib/std/fs/test.zig

Lines changed: 62 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -320,14 +320,8 @@ test "accessAbsolute" {
320320
var tmp = tmpDir(.{});
321321
defer tmp.cleanup();
322322

323-
var arena = ArenaAllocator.init(testing.allocator);
324-
defer arena.deinit();
325-
const allocator = arena.allocator();
326-
327-
const base_path = blk: {
328-
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..] });
329-
break :blk try fs.realpathAlloc(allocator, relative_path);
330-
};
323+
const base_path = try tmp.dir.realpathAlloc(testing.allocator, ".");
324+
defer testing.allocator.free(base_path);
331325

332326
try fs.accessAbsolute(base_path, .{});
333327
}
@@ -338,25 +332,52 @@ test "openDirAbsolute" {
338332
var tmp = tmpDir(.{});
339333
defer tmp.cleanup();
340334

335+
const tmp_ino = (try tmp.dir.stat()).inode;
336+
341337
try tmp.dir.makeDir("subdir");
342-
var arena = ArenaAllocator.init(testing.allocator);
343-
defer arena.deinit();
344-
const allocator = arena.allocator();
338+
const sub_path = try tmp.dir.realpathAlloc(testing.allocator, "subdir");
339+
defer testing.allocator.free(sub_path);
345340

346-
const base_path = blk: {
347-
const relative_path = try fs.path.join(allocator, &.{ ".zig-cache", "tmp", tmp.sub_path[0..], "subdir" });
348-
break :blk try fs.realpathAlloc(allocator, relative_path);
349-
};
341+
// Can open sub_path
342+
var tmp_sub = try fs.openDirAbsolute(sub_path, .{});
343+
defer tmp_sub.close();
344+
345+
const sub_ino = (try tmp_sub.stat()).inode;
350346

351347
{
352-
var dir = try fs.openDirAbsolute(base_path, .{});
348+
// Can open sub_path + ".."
349+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".." });
350+
defer testing.allocator.free(dir_path);
351+
352+
var dir = try fs.openDirAbsolute(dir_path, .{});
353353
defer dir.close();
354+
355+
const ino = (try dir.stat()).inode;
356+
try testing.expectEqual(tmp_ino, ino);
354357
}
355358

356-
for ([_][]const u8{ ".", ".." }) |sub_path| {
357-
const dir_path = try fs.path.join(allocator, &.{ base_path, sub_path });
359+
{
360+
// Can open sub_path + "."
361+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, "." });
362+
defer testing.allocator.free(dir_path);
363+
358364
var dir = try fs.openDirAbsolute(dir_path, .{});
359365
defer dir.close();
366+
367+
const ino = (try dir.stat()).inode;
368+
try testing.expectEqual(sub_ino, ino);
369+
}
370+
371+
{
372+
// Can open subdir + "..", with some extra "."
373+
const dir_path = try fs.path.join(testing.allocator, &.{ sub_path, ".", "..", "." });
374+
defer testing.allocator.free(dir_path);
375+
376+
var dir = try fs.openDirAbsolute(dir_path, .{});
377+
defer dir.close();
378+
379+
const ino = (try dir.stat()).inode;
380+
try testing.expectEqual(tmp_ino, ino);
360381
}
361382
}
362383

@@ -409,10 +430,7 @@ test "readLinkAbsolute" {
409430
defer arena.deinit();
410431
const allocator = arena.allocator();
411432

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

417435
{
418436
const target_path = try fs.path.join(allocator, &.{ base_path, "file.txt" });
@@ -748,7 +766,6 @@ test "directory operations on files" {
748766
test "file operations on directories" {
749767
// TODO: fix this test on FreeBSD. https://github.com/ziglang/zig/issues/1759
750768
if (native_os == .freebsd) return error.SkipZigTest;
751-
if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; // https://github.com/ziglang/zig/issues/20747
752769

753770
try testWithAllSupportedPathTypes(struct {
754771
fn impl(ctx: *TestContext) !void {
@@ -759,18 +776,30 @@ test "file operations on directories" {
759776
try testing.expectError(error.IsDir, ctx.dir.createFile(test_dir_name, .{}));
760777
try testing.expectError(error.IsDir, ctx.dir.deleteFile(test_dir_name));
761778
switch (native_os) {
762-
// no error when reading a directory.
763-
.dragonfly, .netbsd => {},
764-
// Currently, WASI will return error.Unexpected (via ENOTCAPABLE) when attempting fd_read on a directory handle.
765-
// TODO: Re-enable on WASI once https://github.com/bytecodealliance/wasmtime/issues/1935 is resolved.
766-
.wasi => {},
779+
.dragonfly, .netbsd => {
780+
// no error when reading a directory. See https://github.com/ziglang/zig/issues/5732
781+
const buf = try ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize));
782+
testing.allocator.free(buf);
783+
},
784+
.wasi => {
785+
// WASI return EBADF, which gets mapped to NotOpenForReading.
786+
// See https://github.com/bytecodealliance/wasmtime/issues/1935
787+
try testing.expectError(error.NotOpenForReading, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
788+
},
767789
else => {
768790
try testing.expectError(error.IsDir, ctx.dir.readFileAlloc(testing.allocator, test_dir_name, std.math.maxInt(usize)));
769791
},
770792
}
771-
// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
772-
// TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
773-
try testing.expectError(error.IsDir, ctx.dir.openFile(test_dir_name, .{ .mode = .read_write }));
793+
794+
if (native_os == .wasi and builtin.link_libc) {
795+
// wasmtime unexpectedly succeeds here, see https://github.com/ziglang/zig/issues/20747
796+
const handle = try ctx.dir.openFile(test_dir_name, .{ .mode = .read_write });
797+
handle.close();
798+
} else {
799+
// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
800+
// TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
801+
try testing.expectError(error.IsDir, ctx.dir.openFile(test_dir_name, .{ .mode = .read_write }));
802+
}
774803

775804
if (ctx.path_type == .absolute and comptime PathType.absolute.isSupported(builtin.os)) {
776805
try testing.expectError(error.IsDir, fs.createFileAbsolute(test_dir_name, .{}));
@@ -993,10 +1022,7 @@ test "renameAbsolute" {
9931022
defer arena.deinit();
9941023
const allocator = arena.allocator();
9951024

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

10011027
try testing.expectError(error.FileNotFound, fs.renameAbsolute(
10021028
try fs.path.join(allocator, &.{ base_path, "missing_file_name" }),
@@ -1386,7 +1412,6 @@ test "sendfile" {
13861412
defer tmp.cleanup();
13871413

13881414
try tmp.dir.makePath("os_test_tmp");
1389-
defer tmp.dir.deleteTree("os_test_tmp") catch {};
13901415

13911416
var dir = try tmp.dir.openDir("os_test_tmp", .{});
13921417
defer dir.close();
@@ -1451,7 +1476,6 @@ test "copyRangeAll" {
14511476
defer tmp.cleanup();
14521477

14531478
try tmp.dir.makePath("os_test_tmp");
1454-
defer tmp.dir.deleteTree("os_test_tmp") catch {};
14551479

14561480
var dir = try tmp.dir.openDir("os_test_tmp", .{});
14571481
defer dir.close();
@@ -1800,10 +1824,7 @@ test "'.' and '..' in absolute functions" {
18001824
defer arena.deinit();
18011825
const allocator = arena.allocator();
18021826

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

18081829
const subdir_path = try fs.path.join(allocator, &.{ base_path, "./subdir" });
18091830
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)