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

feat: run test through filtered 'zig build test' instead of 'zig test' if build.zig is present. #399

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

omissis
Copy link

@omissis omissis commented Feb 16, 2025

Hi! This PR introduces the ability to run a single test through the Code Lens button leveraging the build.zig file, if present. If not, it will fallback to the previous mechanism.

The reason for this is leveraging the full configuration encoded in the build script, as opposed run on a specific file, which could not work under certain conditions such as importing a file from outside the current module (eg: https://github.com/omissis/zig-langchain/blob/main/src/openai/OpenAI.zig#L2).

By running through zig build test --summary all -- TESTNAME, such test can be run in isolation during development, allowing for a tight feedback loop.

This is probably tackling what asked for in #390

Copy link
Member

@Vexu Vexu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in the linked issue this should use test-unit instead of just test since that may be running much more extensive testing than what we want (see for example the main Zig repo). It should also use a -Dtest-filter argument to actually filter to a specific test.

@kristoff-it
Copy link
Member

kristoff-it commented Feb 18, 2025

I think the best choice is to not hardcode any particular invocation and instead add an entry in the settings that allows users to specify how they want their tests to be invoked.

Ideally this should be a per-project setting, although I'm not sure if and how that's achievable with vscode, but at the very least making it parametric lets users adapt to whatever convention they happen to be using at a given time.

So, in practice, my suggestion would be:

  • default to zig test $file --test-filter=$filter but, on test run failure, prepend a line that reminds the user that if they have different needs, they can fill out a setting field to override the command being run
  • In the field allow the user to specify a format string where they can access the file location and the filter value as variables that they can interpolate in the string as they please
  • bonus points if this can somehow be specified per project

With this system in place people will be able to adapt to whatever convention they prefer:

  • zig build test -Dtest-filter=foo
  • zig build test-unit -- foo (note that in the build script b.args can be used directly to specify filters if the script is configured accordingly)

and so forth.

@Vexu let us know if you have any objection to this approach, pinging also @Techatrix

@Vexu
Copy link
Member

Vexu commented Feb 18, 2025

Sure, that works too. The main codelens should also be updated to have a similar option available.

Ideally this should be a per-project setting, although I'm not sure if and how that's achievable with vscode,

All options can be configured per workspace.

@omissis-sysdig
Copy link

omissis-sysdig commented Feb 19, 2025

So I pushed the change @kristoff-it suggested, which hopefully is a step in the right direction. I am not 100% sure about leaving the lastPart at the end of the args array, it feels a bit clunky and fragile, but it's needed nonetheless.
What I am not 100% convinced about is the usage of a one (and only one) specific build step.

I'd like to clarify with an example (note that I am not super familiar with zig so forgive me if I am saying something silly):

Let's say that I have three categories of tests:

  • test-unit, that need no external dependencies
  • test-integration, that need a postgres database
  • test-expensive, that need to call a bunch of third-party apis to send text messages to ensure our adapters are still working with the providers api. we call these tests once day in our CI when running our nightly pipeline.

For the sake of the example, let's say test-unit are found in the aaa.zig file, test-integration are in the bbb.zig file, and test-expensive are in the ccc.zig file.

I then set my zig.testArgs to ["test-unit", "--"]. I do expect the tests in aaa.zig to succeed when ran through the code lens, but what about the ones in bbb.zig(assuming I have a pg instance running through whatever mean) and ccc.zig? they probably won't be able to build through the test-unit build step, right?

Thanks!

@Vexu
Copy link
Member

Vexu commented Feb 20, 2025

By having the $file argument in the format string you could have a special build step that decides which step to run based on the file path. Something like:

    const special_step = b.step("test-special", "Checks test-path to select proper step");
    const test_path = b.option([]const u8, "test-path", "path to file with the test") orelse "aaa.zig";

    if (std.mem.eql(u8, test_path, "aaa.zig")) {
        special_step.dependOn(test_unit);
    } else if (std.mem.eql(u8, test_path, "bbb.zig")) {
        special_step.dependOn(test_integration);
    } else if (std.mem.eql(u8, test_path, "ccc.zig")) {
        special_step.dependOn(test_expensive);
    } else {
        std.debug.print("invalid test path {s}\n", .{test_path});
        std.process.exit(1);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants