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

Add minimal wasm-bindgen test crate #696

Merged
merged 17 commits into from Jan 22, 2019

Conversation

gridbugs
Copy link
Contributor

@gridbugs gridbugs commented Jan 15, 2019

This adds a simple test of wasm-bindgen support. It uses wasm-pack to compile the test to wasm, and run it in nodejs.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

Thank you @stevebob!

.travis.yml Outdated Show resolved Hide resolved
tests/wasm_bindgen/Cargo.toml Outdated Show resolved Hide resolved
tests/wasm_bindgen/src/lib.rs Show resolved Hide resolved
@gridbugs
Copy link
Contributor Author

I ran into a problem adding OsRng and StdRng::from_entropy tests where the wasm-pack test command failed unexpectedly. I added a simple javascript file which requires the wesm-bindgen'd wasm module, which demonstrates that building the crate produces a module where OsRng and StdRng::from_entropy work as expected.

I've removed the wasm-bindgen-test tests, and instead just test with the javascript file.

Running the js file when the crate is built without the "wasm-bindgen" feature results in the nodejs program crashing (which is expected).

I've made an issue in the wasm-bindgen repo about the test failing when OsRng is used: rustwasm/wasm-bindgen#1189

@dhardy
Copy link
Member

dhardy commented Jan 17, 2019

Good work on tracking that down. Does it mean that using wasm-pack test should work now that the rand_os bug is fixed?

@gridbugs
Copy link
Contributor Author

It should work now. I'll add the test back. Is it worth leaving the current test that runs a little js program as well?

@dhardy
Copy link
Member

dhardy commented Jan 18, 2019

I don't know, whatever you think is most useful!

@gridbugs
Copy link
Contributor Author

I added the wasm-bindgen-test tests back, and kept the existing nodejs script test as well. There's currently a CI test failing, but it doesn't look related to this change. Is there a way to re-run the failed job in case it's a transient problem?

.travis.yml Outdated Show resolved Hide resolved
@dhardy
Copy link
Member

dhardy commented Jan 19, 2019

Yes, I just restarted that job.

@dhardy
Copy link
Member

dhardy commented Jan 20, 2019

My guess is that there's some other reason that test is failing now.

@dhardy
Copy link
Member

dhardy commented Jan 22, 2019

Yes, it is some other reason... merging this.

@dhardy dhardy merged commit 6f3875f into rust-random:master Jan 22, 2019
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.

None yet

2 participants