Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

add testing for wasm #38

Merged
merged 4 commits into from
Jul 23, 2020
Merged

add testing for wasm #38

merged 4 commits into from
Jul 23, 2020

Conversation

Mythra
Copy link
Contributor

@Mythra Mythra commented Jul 16, 2020

fixes #37

add testing in CI for wasm targets. Testing in WASM is a bit weird right now, since you need a wasm environment to actually run in. Also no wasm testing solutions support doctests (and even if they did most of the doctests in color-eyre depend on doing something like reading to STDOUT/STDIN, which most WASM ABIs don't have AFAIK).

As such the following has been done here:

  • Utilize a JS WASM environment tooling: wasm-pack && wasm-bindgen-test.
  • Create a test target they can "see" , adding a test folder, and using: #[wasm_bindgen_test].
    • Have that test validate the bare minimum functionality: installing, printing out an error.
  • Run those tests using NodeJS so we don't have to worry about any browser weirdness.

Hopefully the approach is okay? WASM made it a bit awkward, but I figured may as well start off the PR with what works. Ideally once tracing has added in wasm test support, we can also validate the tracing bit of functionality with another #[wasm_bindgen_test], I figured we shouldn't add that before incase they accidentally break it somehow without meaning to because they aren't testing it.

Mythra added 2 commits July 16, 2020 12:39
utilize wasm-bindgen-test as a developer dependency in order to
actively run wasm tests. wasm currently doesn't have support for
running doctests (and reportedly won't "anytime soon"), and in
order to actually run the tests we need a wasm environment.

the easiest of these to setup & use is wasm-pack which is primarly
built for nodejs. however, if it works in nodejs it should ideally
work in other wasm environments that are similar.

as such a wasm-pack (which utilizies wasm-bindgen) as a developer
dependency has been added that creates a simple error message,
and validates it contains the text of the error (and sections)
sans any formatting. this is meant to validate a base level
of functionality as time goes on.
@Mythra
Copy link
Contributor Author

Mythra commented Jul 16, 2020

Hmm not sure what to do about that clippy warning, as locally I don't seem to receive it (also I didn't touch that file):

$ cargo clippy -- -D warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
$ echo $?
0

Will try looking into it, but unsure.

@yaahc
Copy link
Collaborator

yaahc commented Jul 16, 2020

Yea, that clippy warning is very odd. I don't think its from your change though so don't worry too much about figuring it out.

Let me change the CI settings so clippy isn't required for merge.

@Mythra
Copy link
Contributor Author

Mythra commented Jul 16, 2020

Thanks! I also think the "two expected" that aren't reporting are as a result of me changing the matrix so the job name changed? It's weird they don't have a "details" link.

@yaahc
Copy link
Collaborator

yaahc commented Jul 16, 2020

I think it might be because I still had the jobs with those names as required jobs in CI, I changed that and am restarting CI, lets see if it works!

@yaahc
Copy link
Collaborator

yaahc commented Jul 16, 2020

I'm gonna leave this open for now because I don't have any other changes for color-eyre planned short term so there wont be any missed testing. I'll try to figure out the clippy issue. This is already good to go as far as I can tell though, so I'll merge this as soon as I need it or once the clippy thing is fixed. Thanks again for setting this up!

@Mythra
Copy link
Contributor Author

Mythra commented Jul 16, 2020

No problem at all, thanks for your review and the help!

@yaahc yaahc merged commit 1df9ec5 into eyre-rs:master Jul 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for WASM: Add CI for wasm32-unknown-unknown
2 participants