github actions build and test on major architectures #817
Conversation
… into github-actions
ignore windows gnu failing tests
|
This looks amazing. I'm not going to formally review it until it's all green and you tell me it's ready for review, but thanks a ton for all this! Proper amount of effort to get all those fiddly details right, cheers! |
tighten scope so that workflows only run on pushes to main turn on clippy for PR runs instead of just scheduled runs
no longer needed as the cmd_test is correctly marked as an integration test, so that its binaries are built before tests are run
This ensure that cargo builds its required binaries before running the test, which is a requirement for these tests
reduce_test should be an integration test, as it depends upon its binaries to run tests This change lets `cargo test` work without a prior `cargo build`
| @@ -12,13 +12,15 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
|
|
|||
| use crate::test_utils::{ | |||
| mod test_utils; | |||
There was a problem hiding this comment.
JFYI I am likely to use these test_utils to implement an mdbook doctest macro per #797.
There was a problem hiding this comment.
JFYI I am likely to use these
test_utilsto implement an mdbook doctest macro per #797.
Thanks for the heads up. I turned it back into draft because I'm seeing flaky integration tests on some of the architectures. I don't know the cause. A possible cause could be a compiler bug (and in fact the error messages literally always say that it's a compiler bug). However, before giving up on having reliable non-compiler-crashing integration tests, I thought I'd like to see if I could get the tests to run just via cargo test without a cargo build needed first. That means turning the tests that need binaries to work into Rust "integration tests" and that's the work you can see being partially done here.
I wish I could do it all locally, but I haven't got a way of running github actions across multiple architectures locally.
I can reproduce the flaky test failures on beta and nightly Rust channels locally. Anyway, that's where it's at.
Obviously if you merge #797 first I will have to deal. Otherwise we'll find some way of sharing the code that needs to be shared between integration tests and the mdbook, I'm sure.
separate lists for: * windows (msvc) * windows (gnu) * beta Make integration tests into integration tests. This lets `cargo test` succeed, as it then builds the needed dev-dependencies before running the tests
|
Yep! Also, consider splitting this into two PRs? If there are some platforms which work reliably let's get them merged! |
add the msvc flag that turns on exception handling to builds try building chromium on Windows with exceptions
hard-code the clang c++14 flag again Seems that using `flag_if_supported` interacts poorly with the code in the `test_stringview` test that sets c++17. Look at in a future PR
All known issues are dealt with. Issues raised to triage further post merge |
|
@adetaylor this is ready for review and potential merge |
| @@ -382,7 +384,8 @@ where | |||
| .host(&target) | |||
| .target(&target) | |||
| .opt_level(1) | |||
| .flag("-std=c++14"); | |||
| .flag("-std=c++14") // For clang | |||
There was a problem hiding this comment.
Should this be flag_if_supported? It seems surprising this works! Obviously, it does though...
| [[test]] | ||
| name = "integration_tests" | ||
| path = "tests/lib.rs" | ||
| harness = true |
There was a problem hiding this comment.
Please could you add a newline at the end?
|
In fact I just went ahead and merged this - I'll raise a PR to add the missing newline as a way of testing this. |
Fixes #796
Adds test and examples workflows for ubuntu-latest (stable, beta, nightly), macos nightly, windows nightly (msvc and gnu), tighter clippy, and outdated package notification
autocxx-reduceandautocxx-genexcluded on Windows targets ascreduceisn't packaged theres2andchromium-fake-render-frame-hostexamples fail to link on Windows, so excluded thereclippycheck is made strongeroutdatedcheck will generally fail. It should be used for notification, not as a PR-merge requirement