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

Move CI to GitHub Actions #1474

Merged
merged 38 commits into from
Aug 15, 2020
Merged

Move CI to GitHub Actions #1474

merged 38 commits into from
Aug 15, 2020

Conversation

siku2
Copy link
Member

@siku2 siku2 commented Aug 7, 2020

The pull request checks are the only thing that's still using Travis. This PR moves them to GitHub Actions and expands the checks a bit.

Notable Changes

  • Examples are checked again (cargo check)
  • Lints (clippy, rustfmt) use the nightly toolchain. This should help us stay ahead of the curve.
  • Integration tests run on Chrome too (Chrome seems to break a lot, we might need to remove it again)

Benefits:

  • Makes it easier to pinpoint an issue
  • Faster thanks to parallel tasks
  • Should be more stable than Travis

I left run_stable_checks.sh and run_tests.sh for now because they're useful to contributors.
The changes uncovered some new Clippy issues which explains the remaining changes in this PR.

Potential issues

1

I didn't port `cargo test --doc --features doc_test,wasm_test,yaml,msgpack,cbor,toml --features std_web,agent,services --no-default-features`. I don't think this adds anything to the existing tests since it just activates all default features manually.

2

The following part of run_stable_checks.sh is missing:

pushd yew-stdweb
# webgl_stdweb doesn't play nice with wasm-bindgen
(cd examples/webgl && cargo web check --target wasm32-unknown-unknown)
popd

It's the only part that requires cargo-web. Since it's just a single example in yew-stdweb, I don't think it's really necessary.

3

Not sure whether Mergify can deal with this because it's no longer just a single check and Mergify's documentation mentions something about this: https://doc.mergify.io/conditions.html#validating-all-status-check.

@siku2 siku2 added the A-ci Area: The continuous integration label Aug 7, 2020
@siku2 siku2 requested a review from jstarry August 7, 2020 15:10
@jstarry
Copy link
Member

jstarry commented Aug 15, 2020

I didn't port cargo test --doc --features doc_test,wasm_test,yaml,msgpack,cbor,toml --features std_web,agent,services --no-default-features. I don't think this adds anything to the existing tests since it just activates all default features manually.

The intent was for this to run tests on yew-stdweb (need to disable "web-sys" feature)

It's the only part that requires cargo-web. Since it's just a single example in yew-stdweb, I don't think it's really necessary.

Fine with this

Not sure whether Mergify can deal with this because it's no longer just a single check and Mergify's documentation mentions something about this: https://doc.mergify.io/conditions.html#validating-all-status-check.

Not too concerned about Mergify. It's a nice to have but this PR makes things so much better that there's less need for mergify :)

Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

This is a huge improvement! Nice that you setup both chrome / gecko driver tests 👍

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
@siku2
Copy link
Member Author

siku2 commented Aug 15, 2020

The intent was for this to run tests on yew-stdweb (need to disable "web-sys" feature)

Oh right, I totally didn't notice the std_web feature. I added the test for yew-stdweb back in.

@siku2
Copy link
Member Author

siku2 commented Aug 15, 2020

Alright, that should be everything! Only took me like 3 hours 😭 ...

@jstarry
Copy link
Member

jstarry commented Aug 15, 2020

Lol, that's CI for ya. Really appreciate it!

@jstarry jstarry merged commit 55f7a42 into yewstack:master Aug 15, 2020
jstarry added a commit that referenced this pull request Aug 16, 2020
* clippy in tests

* add pull request workflow

* fix formatting

* add names to steps

* fix clippy

* update

* avoid mutable

* use wasm target

* install it too

* except there

* let's give chrome a spin

* clippy has some more to say

* only run specific tests

* fix tests on chrome

* add cache

* rename benchmark for consistency

* re-enable geckodriver

* clean up old files

* remove all mentions of Travis

* check all examples

* let's try with a bigger timeout

* test both browsers at the same time

* chrome really doesn't like me

* finish up

* run for pushes to master

* improve caching

* fix order in workspace members

* clippy use --all-targets

* rename workflow file

* Apply suggestions from code review

Co-authored-by: Justin Starry <[email protected]>

* use stable toolchain for lints

* run lints on nightly too

* add doctest for yew-stdweb

* allow failure for nightly clippy steps

* let's try it on the job level again

* always run all lint steps

* only run lints on stable toolchain

* use 'no_run' instead of 'ignore'

Co-authored-by: Justin Starry <[email protected]>
teymour-aldridge pushed a commit to teymour-aldridge/yew that referenced this pull request Sep 1, 2020
* clippy in tests

* add pull request workflow

* fix formatting

* add names to steps

* fix clippy

* update

* avoid mutable

* use wasm target

* install it too

* except there

* let's give chrome a spin

* clippy has some more to say

* only run specific tests

* fix tests on chrome

* add cache

* rename benchmark for consistency

* re-enable geckodriver

* clean up old files

* remove all mentions of Travis

* check all examples

* let's try with a bigger timeout

* test both browsers at the same time

* chrome really doesn't like me

* finish up

* run for pushes to master

* improve caching

* fix order in workspace members

* clippy use --all-targets

* rename workflow file

* Apply suggestions from code review

Co-authored-by: Justin Starry <[email protected]>

* use stable toolchain for lints

* run lints on nightly too

* add doctest for yew-stdweb

* allow failure for nightly clippy steps

* let's try it on the job level again

* always run all lint steps

* only run lints on stable toolchain

* use 'no_run' instead of 'ignore'

Co-authored-by: Justin Starry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: The continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants