Stream: git-wasmtime

Topic: wasmtime / PR #5287 Split CMake tests into a separate step

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:23):

elliottt opened PR #5287 from trevor/remove-cmake-from-test to main:

Split the CMake tests for exercising the c-api out into a separate step. There
doesn't appear to be any reuse between build artifacts from the CMake-based
tests and the cargo test step later on in the Test job, so this should speed
up the average build time.

Additionally, this PR only runs the CMake tests on ubuntu and windows. I'd be
happy to expand the matrix out to the same set as the Test job if we think
it's worth it.
<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:41):

elliottt requested alexcrichton for a review on PR #5287.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:45):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:45):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:45):

jameysharp created PR review comment:

I suppose you could remove the matrix.target bits since this job doesn't try cross-compiling anything. Anyone wanting to test the C API on, say, riscv would need to copy all the qemu machinery from the other job, so expecting them to also add matrix.target in appropriate places seems okay too.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:49):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:49):

elliottt created PR review comment:

Yeah, good point!

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:50):

elliottt updated PR #5287 from trevor/remove-cmake-from-test to main.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:53):

elliottt has marked PR #5287 as ready for review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:57):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:57):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:57):

alexcrichton created PR review comment:

It's ok to skip these steps since that's only required for one builder to do it.

Wasmtime GitHub notifications bot (Nov 17 2022 at 00:57):

alexcrichton created PR review comment:

I know windows is weird but we're tight enough on CI budget I'd be ok to drop windows here and just test on Linux

Wasmtime GitHub notifications bot (Nov 17 2022 at 01:14):

elliottt updated PR #5287 from trevor/remove-cmake-from-test to main.

Wasmtime GitHub notifications bot (Nov 17 2022 at 05:23):

elliottt updated PR #5287 from trevor/remove-cmake-from-test to main.

Wasmtime GitHub notifications bot (Nov 17 2022 at 06:04):

elliottt updated PR #5287 from trevor/remove-cmake-from-test to main.

Wasmtime GitHub notifications bot (Nov 17 2022 at 17:30):

elliottt closed without merge PR #5287.

Last updated: Nov 22 2024 at 16:03 UTC