tests: test unit tests #3136
tests: test unit tests #3136
Conversation
cb22
force-pushed
the
cb22/forget-me-not
branch
3 times, most recently
from
f530d7c to
1a795dd
Compare
build.zig
Outdated
| var unit_test_files = std.ArrayList([]const u8).init(b.allocator); | ||
| defer unit_test_files.deinit(); | ||
| defer for (unit_test_files.items) |item| b.allocator.free(item); |
There was a problem hiding this comment.
I'd make a local area here, to not bother with freeing individual items.
build.zig
Outdated
| defer b.allocator.free(contents); | ||
|
|
||
| if (std.mem.indexOf(u8, contents, "test \"") == null and | ||
| std.mem.indexOf(u8, contents, "test {") == null) |
There was a problem hiding this comment.
Zig also allows for
fn foo() void {}
test foo {}
Maybe check that trimmed line starts with test ?
There was a problem hiding this comment.
Good catch, this missed the test in forest.zig.
build.zig
Outdated
| !std.mem.startsWith(u8, entry_path, "clients/c")) continue; | ||
| if (std.mem.eql(u8, entry_path, "tigerbeetle/libtb_client.zig")) continue; | ||
|
|
||
| const contents = try src_dir.readFileAlloc(b.allocator, entry_path, 1024 * 1024); |
There was a problem hiding this comment.
Hm, so this effectively slurps the entire source directory at the configure time. I think we'd love to avoid that, as this moves our no-op build performance from O(number of files) to O(total length of files).
Oh! Haha! let's do a snapshot test here! Inside unit_tests.zig, we can add a test that, when run, generates the source code of the unit_tests.zig file, fails if it doesn't match the actual source code, and updates itself.
|
As per new review policy, pushed some suggestions directly! |
build.zig
Outdated
|
|
||
| const build_root = b.addOptions(); | ||
| build_root.addOption([]const u8, "build_root", b.build_root.path.?); | ||
| unit_tests.root_module.addOptions("build_root", build_root); |
There was a problem hiding this comment.
Maybe use std.fs.cwd()? That's what snaptest is doing, and it's been reliable for me!
tigerbeetle/src/stdx/testing/snaptest.zig
Line 200 in 2496feb
There was a problem hiding this comment.
Maybe use std.fs.cwd()? That's what snaptest is doing, and it's been reliable for me!
If you run ./zig/zig test and not eg ../zig/zig build from src/, which is currently broken anyways for test (but works for normal building!)
There was a problem hiding this comment.
Hm, it definitely does use fixed cwd as of 0.14.1:
However, this seems to be changing for 0.15:
I guess in any case we can use
run_unit_tests.setCwd(b.path("."));
Injecting absolute path in the binary feels weird, as it makes the actual binary not-reproducible between machines.
There was a problem hiding this comment.
Oh hmm, must be something else cause this then:
~/g/t/tigerbeetle-3/src cb22/rename-nodes• 1.3s | 130 ❱ ../zig/zig build test
test
└─ test:integration
└─ run test
└─ zig test Debug x86_64-linux
└─ options
└─ zig build-exe vortex Debug x86_64-linux 2 errors
testing/vortex/zig_driver.zig:10:11: error: C import failed
const c = @cImport({
^~~~~~~~
referenced by:
main: testing/vortex/zig_driver.zig:38:20
main: vortex.zig:59:52
main: /tank/mutable/synced/git/tigerbeetle/tigerbeetle-3/zig/lib/std/start.zig:660:37
comptime: /tank/mutable/synced/git/tigerbeetle/tigerbeetle-3/zig/lib/std/start.zig:58:30
start: /tank/mutable/synced/git/tigerbeetle/tigerbeetle-3/zig/lib/std/std.zig:97:27
comptime: /tank/mutable/synced/git/tigerbeetle/tigerbeetle-3/zig/lib/std/std.zig:168:9
/tank/mutable/synced/git/tigerbeetle/tigerbeetle-3/.zig-cache/o/8cbe1d82feb80658a77104a8d02d3f13/cimport.h:1:10: error: 'tb_client.h' file not found
...
run_unit_tests.setCwd(b.path("."));
Ah yes, much nicer! Done, and rebased.
| if (std.process.hasEnvVarConstant("SNAP_UPDATE")) { | ||
| const includes_end = std.mem.indexOf(u8, unit_tests_contents_disk, "}\n").? + 2; | ||
|
|
||
| // Add the rest of the real file on disk to the generated in-memory file. |
There was a problem hiding this comment.
I am wondering if we should turn this into a true quine which doesn't read its own source code, just for cool points 🙈
Inspired by #3124 (comment) this generates the `unit_tests.zig` file in `build.zig`. There's a bit of logic as to what files should be included, so I thought it's more evident to go with a generated in-repo file rather than removing `unit_tests.zig` entirely. Another option could be to keep `unit_tests.zig` as manually generated but lint it to ensure everything with a `test` block is included.
cb22
force-pushed
the
cb22/forget-me-not
branch
from
4d7ab3e to
1fbbff5
Compare
|
failed: src/unit_tests.zig: error: 'build_root' is dead code |
cb22
force-pushed
the
cb22/forget-me-not
branch
from
1fbbff5 to
2c87d9b
Compare
|
Oops, sorted. |
2 participants
Inspired by #3124 (comment) this generates the
unit_tests.zigfile inbuild.zig.There's a bit of logic as to what files should be included, so I thought it's more evident to go with a generated in-repo file rather than removing
unit_tests.zigentirely.Another option could be to keep
unit_tests.zigas manually generated but lint it to ensure everything with atestblock is included.