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

Implement brioche test subcommand #94

Open
kylewlacy opened this issue Jul 17, 2024 · 2 comments
Open

Implement brioche test subcommand #94

kylewlacy opened this issue Jul 17, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@kylewlacy
Copy link
Member

Currently, the only way to validate if a Brioche package is working correctly is to try building and running it. This has a few problems:

  1. Not every package is runnable. The std package and packages that just wrap a library don't have an "obvious" way to get run... at least not one that would be usable via the brioche run command line
  2. For packages that are runnable, it's not always clear how to minimally test them. At least one of --version, version, --help, or help would work, but that's not guaranteed, and there's no way to know which
  3. Using brioche run runs on the host machine. This means its possible for a package to work with brioche run but to fail when used within Brioche, e.g. by calling it with std.runBash

As a workaround, a few packages (namely, openssl, plus bat in #70) expose an export named test. This can be used explicitly by running brioche build -e test, which allows for writing arbitrary assertions that can run in the Brioche sandbox natively. But, just using a test export has its own downsides:

  • Hard to automate: we'd ideally like to run tests in CI/CD, but to pass if no test is defined. There's no clean way to do this today
  • Somewhat unintuitive to write: because test is just a normal export, it must return a recipe that gets baked. Plus, that recipe must also write to $BRIOCHE_OUTPUT explicitly, otherwise the build will fail.

A solution to both problems is to add a new brioche test command, specifically designed for writing tests. We also don't need to be bound by the same rules that exports follow today.

To start with, we should focus on writing tests for packages, as this is the clearest need. At some point, it could make sense to expand the scope and offer features that help users write tests more generally for their own projects

@kylewlacy
Copy link
Member Author

I think mirroring a common testing library would work pretty well for Brioche:

import * as std from "std";
import * from "test";

export const project = {
  name: "curl",
  version: "1.23.45",
};

export default function curl() { /* ... */ }

describe("curl", () => {
  it("has the right version", async () => {
    const expectedVersion = project.version;
    const actualVersion = await std.runBash`curl --version > "$BRIOCHE_OUTPUT"`.dependencies(curl()).toFile().read();
    expect(expectedVersion).toEqual(actualVersion);
  });
});

In this case, I went with a syntax like Ruby RSpec (that's at least where I first saw this style of test, and it's been copied for testing frameworks like Jest and Vitest in the JS ecosystem)

We could also go with a simpler model, more like what Rust does:

import * as std from "std";

std.test("test curl version", async () => {
  const expectedVersion = project.version;
  const actualVersion = await std.runBash`curl --version > "$BRIOCHE_OUTPUT"`.dependencies(curl()).toFile().read();
  std.assert(expectedVersion == actualVersion);
});

(and of course, either choice could have all sorts of utilities for more complex assertions or helpers for getting the output from a Bash command)


The key is that describe or std.test or whatever would use some magic runtime features to pass the test function back to the runtime, so it would make a call like Deno.core.ops.op_register_test("test curl version", testFn);

Then, the runtime would either run or skip the test, depending on if the project is being called with brioche test or not. This would also mean you could define tests anywhere in the project, as long as the module gets imported somewhere. To me, this seems more flexible over having a single top-level test export

@kylewlacy
Copy link
Member Author

Actually, I'm pretty split here. Using a simple test export would definitely be easier to implement and would be more consistent with how normal builds work in Brioche. Plus, it would still be possible to build more complicated test runners by just using normal TypeScript features, meaning it exists in "user space" instead of natively in the runtime (in other words, we could build the above system on top of a single call like export function test() { return runAllTests(); }, where the "test registry" is handled by runAllTests)

At least as an initial implementation, it seems like just using a test export would give more wiggle room to experiment

@kylewlacy kylewlacy added the enhancement New feature or request label Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant