Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zig test needs a way to run tests for multiple files #10018

Open
Jarred-Sumner opened this issue Oct 24, 2021 · 4 comments
Open

zig test needs a way to run tests for multiple files #10018

Jarred-Sumner opened this issue Oct 24, 2021 · 4 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Jarred-Sumner
Copy link
Contributor

Jarred-Sumner commented Oct 24, 2021

Currently, if you pass multiple files to zig test, it fails with this error:

error: found another zig file 'src/json_parser.zig' after root source file 'src/js_parser.zig'

Say you have many files with tests in them, and you want to run all the tests for those files.

For test runners in other languages, the command for running tests in one file is the same for running tests in several files.

Rust:

# Patterns work:
cargo test --test '**/*.rs'

# --workspace runs all
cargo test --workspace

Go:

# "..." says "recursively run across all subpackages"
go test ./...

JavaScript:

# jest accepts multiple files as input
jest **/*.js

Why not zig test main.zig?

You can run zig test on the entry point, but it's a footgun.

  • zig test silently skips tests in files not referenced by runtime code. That means it reports all tests passed without necessarily running all the tests. This is probably an unintentional side effect of zig's fantastic dead code elimination.
  • It cannot filter tests to a specific file. That means prepending some prefix to test names manually

Why not zig build test?

  • No standardization: every project will have a slightly different implementation with different behavior and bugs.
  • zig build cache invalidates frequently, making it several seconds slower than manually invoking zig test on the target file
  • Two ways to run tests is more confusing than one way.
  • zig build sounds like it's for building, not for testing.

Proposal

Part 1

Support multiple file paths as input in zig test:

zig test src/file1.zig src/file2.zig

This would run the tests in those files.

Note: I am not suggesting a glob matcher. It would be nice, but the shell can do that.

Part 1A

Change the default behavior of zig test to only run test { } and test ".*" { } for the input file(s), instead of all files referenced recursively.

zig test src/file1.zig

This would only run the test blocks defined in src/file1.zig. If src/file1.zig imports another file with test blocks in them, those tests would not be run.

This change would:

  • Make it easier to use tests for development. If I'm passing zig test src/file1.zig, I don't want to see test failures or build failures from tests in src/strings.zig.
  • Make it easier to choose what parts of the test suite are run

The current way to do this is by adding a prefix to every test name and then passing that prefix to zig test. Humans shouldn't have to add a prefix to every test's name.

Part 1B

Being able to run all tests for an entry point is very useful.

Rather than removing that, this proposal suggests changing it from the default behavior to a flag.

Here are two ideas for flag names that come to mind:

# Option 1
zig test --all src/main.zig

# Option 2
zig test -r src/main.zig
zig test --recursive src/main.zig

Part 2

Add support for build.zig to specify default options for zig test, without wrapping zig test as zig build test

Why? Currently, projects with dependencies can't run zig test without adding a lot of flags.

My current incantation for running a test looks something like this:

zig test src/http_client.zig --test-filter "send - redirect" src/deps/libcrypto.a src/deps/libs2n.a -lc -lc++ /Users/jarred/Code/bun/src/deps/zlib/libz.a src/deps/mimalloc/libmimalloc.a  --cache-dir /Users/jarred/Code/bun/zig-cache --global-cache-dir /Users/jarred/.cache/zig --name bun --pkg-begin clap /Users/jarred/Code/bun/src/deps/zig-clap/clap.zig --pkg-end --pkg-begin picohttp /Users/jarred/Code/bun/src/deps/picohttp.zig --pkg-end --pkg-end -I /Users/jarred/Code/bun/src/deps -I /Users/jarred/Code/bun/src/deps/mimalloc -I /usr/local/opt/icu4c/include -L /usr/local/opt/icu4c/lib --main-pkg-path /Users/jarred/Code/bun --enable-cache -femit-bin=zig-out/bin/test src/deps/*.a -liconv

This proposal does not go into the details of what the API would look like for build.zig to specify default options for zig test.

Summary

If this entire proposal were to be implemented:

# Run all tests in project (via build.zig passing configuration)
zig test

# Run tests for a specific file
zig test src/strings.zig

# Run tests for a folder
zig test src/http/*.zig

# Run tests for an entry point
zig test -r src/main.zig

# Run tests matching a pattern in a specific file
zig test --test-filter "replace" src/strings.zig 

Other

This is the script I currently use to generate a file that imports all files with tests, regardless of whether the test files are referenced by runtime code in the real entry point.

#!/bin/bash

cd src
rg -l "^test " --type zig | sed  -e 's/\(.*\)/@import\(\".\/\1"\);/' > /tmp/tests.zig
awk '{printf "const Test%d = %s\ntest { const Foo = Test%d; }\n", NR, $0, NR}' < /tmp/tests.zig > tests.zig
zig fmt tests.zig
@matu3ba
Copy link
Contributor

matu3ba commented Oct 25, 2021

  1. Can you elaborate what parts of this are not redundant to implementing tests (for groups of code) inside zig build ?

  2. What would be the best practice to communicate to new contributors how things must be tested, if they fix a local bug (without having looked into the build system)? As of now, there is no convention.

No standardization: every project will have a slightly different implementation with different behavior and bugs.

Every project has different constraints on logical dependency of how tests are build. Think of tests requiring external dependencies, fetching up-to-date data from the system etc. This point is more of a question, how this should be communicated.

zig build cache invalidates frequently

This should be (de)motivated by a use case. I dont understand the performance implications for different project sizes. Maybe you can also argue, how this can not be fixed in the caching system?

Two ways to run tests is more confusing than one way.

That is a point against using zig test, as zig test cant enforce updating+building project build dependencies etc. Projects can also have special test logic (ie complete repos/subprojects for testing complicated stuff). (Remind: The package manager is for fetching packages.)

zig build sounds like it's for building, not for testing.

Many projects invoke automatic tests after building or making sure dependencies are build etc. Take arocc.

Sidenode: Tests are also build, but then "run". One could distinguish here within the build system, but in practice users expect them to be run immediately on default.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Nov 20, 2021
@andrewrk andrewrk added this to the 0.10.0 milestone Nov 20, 2021
@leighmcculloch
Copy link

leighmcculloch commented Mar 14, 2022

Two ways to run tests is more confusing than one way.

That is a point against using zig test, as zig test cant enforce updating+building project build dependencies etc. Projects can also have special test logic (ie complete repos/subprojects for testing complicated stuff). (Remind: The package manager is for fetching packages.)

As someone who is new to Zig having two ways to run tests has been confusing, but I understand why both exist. It does seem like zig test can't service all the use cases that zig build test satisfies, since projects can have more complex setups that the build.zig defines and running zig test would ignore all that setup. However, zig test also has features and options that zig build steps that contain tests don't get, like command line options for filtering, etc.

It occurs to me that if we made it easier to replicate all the zig test functionality in a build step, with maybe a standard set of options that could be attached to a test build step, and if those set of options were expanded to include filters or selectors like the proposal specifies, then zig build test would be a better default option.

@codethief
Copy link

codethief commented Feb 21, 2023

I'm very much in favor of the entire proposal! I think the fact that there are entire blog posts and threads on Reddit about zig test not running all tests, clearly indicates that there is a demand and there needs to be done something.

@tauoverpi
Copy link
Sponsor Contributor

It would be nice if zig test or zig build test could explain why some of the tests weren't run (e.g "not reached" or "not evaluated") given that zig still has to parse all files and is aware of all tests present within them.

This would likely reduce the confusion when it looks as if only three tests were picked up when you have 30 or so tests in other files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants