Skip to content

Improve CI for web targets - #108

Merged
josephlr merged 5 commits into
rust-random:masterfrom
josephlr:ci
Sep 21, 2019
Merged

Improve CI for web targets#108
josephlr merged 5 commits into
rust-random:masterfrom
josephlr:ci

Conversation

@josephlr

@josephlrjosephlr commented Sep 20, 2019

Copy link
Copy Markdown
Member

This PR uses simplifies the testing story for getrandom. All targets now use the same testing file (eliminating the need for a separate tests/wasm_bindgen crate).

We also use .cargo/config to allow users to run cargo test normally for wasm32-wasi or wasm32-unknown bindgen. This just requires that wasmtime or wasm-bindgen-test-runner exist in the user's path.

The CI for these targets has been overhauled:

  • Tests are now actually run a larger number of targets:
    • wasm32-wasi
    • stdweb: Node and Chrome
    • wasm-bindgen: Node, Firefox, and Chrome
  • Tests are built/linked for asmjs-unknown-emscripten
  • We download pre-built binaries for all our dependancies

Results:

  • Less code
  • Less duplication
  • More tests are run
  • WASM CI is much faster. Decreased from 13 mins to 4.5 mins

This is not part of #98, so it can be merged into master.

Run WASI tests, and run both stdweb/wasm-bindgen on both node and in browsers
These changes also remove the need for a separate tests/wasm_bindgen

@newpavlovnewpavlov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, but I would wait for @dhardy to review it as well.

Comment threadCargo.toml Outdated
@josephlr
josephlrforce-pushed the ci branch 5 times, most recently from 92e5ff3 to f6c8766CompareSeptember 20, 2019 06:19
@josephlrjosephlr changed the title Simplify CI process for WASMImprove CI for web targetsSep 20, 2019
@josephlr

Copy link
Copy Markdown
MemberAuthor

Updated description to reflect the scope of these changes.

@dhardy this is ready, the CI runs significantly faster now.

@dhardy

Copy link
Copy Markdown
Member

I would prefer that someone more familiar with WASM could review. @stevebob wrote this test originally (rand#696).

@dhardydhardy mentioned this pull request Sep 21, 2019

@dhardydhardy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@josephlr
josephlr merged commit fa6f929 into rust-random:masterSep 21, 2019
@josephlr
josephlr deleted the ci branch September 21, 2019 08:46
@josephlrjosephlr mentioned this pull request Oct 1, 2019
takumi-earth pushed a commit to earthlings-dev/getrandom that referenced this pull request Jan 27, 2026
* Simplify CI process for WASM
Run WASI tests, and run both stdweb/wasm-bindgen on both node and in browsers
These changes also remove the need for a separate tests/wasm_bindgen
* Use all pre-built binaries in the CI
* Fix Cargo.toml typo
Co-Authored-By: Artyom Pavlov <newpavlov@gmail.com>
* Fix install location
* Debug CI
Sign up for freeto 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.

3 participants

@josephlr@dhardy@newpavlov