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

trunk test #20

Open
thedodd opened this issue Aug 27, 2020 · 5 comments
Open

trunk test #20

thedodd opened this issue Aug 27, 2020 · 5 comments
Labels
cli Trunk CLI application discussion This item needs some discussion needs design This item needs design work

Comments

@thedodd
Copy link
Member

thedodd commented Aug 27, 2020

abstract

Lots of discussion to be had here. The main idea is that we want a trunk test command which will do the right thing in terms of running unit tests, integration tests &c.

A few ideas were being tossed around on the Yew community Discord. Right now, I am definitely leaning towards leveraging wasm-pack test as much as possible. It already handles a lot of the heavy lifting.

Currently, this issue is in discussion phase. We need to determine what would be best for users.

@thedodd thedodd added help wanted Extra attention is needed cli Trunk CLI application core discussion This item needs some discussion labels Aug 27, 2020
@thedodd thedodd changed the title trunk test trunk test Aug 27, 2020
@thedodd thedodd added needs design This item needs design work and removed core labels Sep 4, 2020
@thedodd thedodd removed the help wanted Extra attention is needed label Oct 2, 2020
@flosse
Copy link

flosse commented May 1, 2021

Are there any plans to go further with this discussion?
wasm-pack is currently not maintained and it would be awesome to have s.th. like
trunk watch -t -h firefox where t = test and h = headless that runs the tests automatically :)

@hamza1311
Copy link
Contributor

I've been playing around with testing without wasm-pack in yewstack/yew#2528. trunk test command could simply do the following:

We would need to ask users to ensure that their .cargo/config has the test runner configured though: https://rustwasm.github.io/wasm-bindgen/wasm-bindgen-test/usage.html#configure-cargoconfig-to-use-the-test-runner

@hamza1311
Copy link
Contributor

I took a shot at implementing it but there's design discussion left to be done:

  • How will we ensure that .cargo/config.toml is preset and properly setup? Do we just ignore that? Do we manually pull wasm-bindgen-test-runner and use it (it's not published in the wasm-bindgen releases)
  • How will we locate the Cargo.toml file? For build command, we can easily use the index.html. But if we do the same but test, we would be preventing trunk from being used to test libraries. My main use case for this was to use trunk to test Yew, which I ended up doing with wasm-bindgen-cli directly (see Don't use wasm-pack in CI yewstack/yew#2648 and Wasm doc tests yewstack/yew#2651). Also, integration tests (tests/ directory) requires a library crate to be present.
  • What about doc tests?

@dnaka91
Copy link
Contributor

dnaka91 commented May 3, 2022

Thanks for trying out to implement this. I think this is a rather big change, so I'll go ahead and implement this. Regarding the issues you faced, I think we can solve most of them rather easy.

Easy to solve

  • We don't need to care about .cargo/config.toml, as we can use env vars to override the runner. You can see here, that CARGO_TARGET_<triple>_RUNNER exists. We can set CARGO_TARGET_wasm32-unknown-unknown_RUNNER=wasm-bindgen-test-runner and should be good to go.
    • Eventually, it's worth to parse a minimal version of the Cargo config file, both project local and global one, and print a warning if the runner is already set.
  • The wasm-bindgen-test-runner is released as pre-compiled binary, as part of the wasm-bindgen releases. We just don't extract it currently.
    • Need to expand our tool download logic, to share the same download archive for the wasm-bindgen-cli and the test runner, so we wouldn't download the same thing twice.
  • The browser drivers can probably handled the same way as we do with other tools, downloading pre-compiled binaries automatically if system-wide installed versions don't exist.
    • Might just start with system-only binaries as a starter and add the auto-download later on.
    • Download would only work for the Firefox and Chrome driver, Safari doesn't have anything we could download ourselves.
    • Downloading the driver still doesn't ensure the browser is installed in the right version, too. That's more of a problem with Chrome, but not that bad with Firefox as the driver runs with many versions.
    • Have to look a bit at wasm-pack to find out how they run the Node.js variant. I guess the test runner does that by itself and just uses the node binary if available, failing otherwise.

Problematic

  • Locating the Cargo.toml file is probably rather easy, but completely different from the build command. At the same time, we want to support binaries too. So we'd need some hybrid solution that works for projects with and without an index.html.
  • Could you share some more details about doc tests? My assumption would have been that cargo runs them the same way as the other tests (plus the wrapping and stuff to run the snippets as tests of course). Have you discovered some differences or problems when running doc tests?

@grodin
Copy link

grodin commented Feb 3, 2023

It seems that wasm-pack is being maintained again so I just wanted to gently bump this issue because it seems that the original plan of leaning on wasm-pack might be reasonable again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Trunk CLI application discussion This item needs some discussion needs design This item needs design work
Projects
None yet
Development

No branches or pull requests

5 participants