update tests to use cargo-nextest if present #1243
Conversation
ermalkaleci
added
ci
ermalkaleci
requested review from
shaunxw,
Dinonard,
ashutoshvarma and
PierreOssun
Makefile
Outdated
|
|
||
| .PHONY: test | ||
| test: | ||
| SKIP_WASM_BUILD= ${cargo_test} --all |
There was a problem hiding this comment.
I think we want to skip --all since it disables some part of the code (even though I think they're not tested by UTs) that are run outside of evm-tracing context. This is why we specified try-runtime,runtime-benchmarks explicitly before.
There was a problem hiding this comment.
actually --all is deprecated and needs to be replaced with --workspace. I don't understand it disables some part of the code
There was a problem hiding this comment.
Some parts of the client code are compiled only when evm-tracing is disabled, and vice-versa. That's what I meant by disabled - poor choice of words from my side.
Since non-evm-tracing client is more important, running tests with disabled evm-tracing makes more sense. But then again, I don't believe we have tests covering client so perhaps this worry is baseless.
There was a problem hiding this comment.
We don't run any test with feature evm-tracing. Will add one
Makefile
Outdated
| .PHONY: test-all | ||
| test-all: test test-runtimes | ||
| SKIP_WASM_BUILD= ${cargo_test} --workspace --features try-runtime,runtime-benchmarks |
There was a problem hiding this comment.
To make my comment more specific about features...
Why not just run it once with features enabled?
I assume this is because you want to ensure that tests work with no features enabled?
bin/collator/src/local/chain_spec.rs
Outdated
| @@ -194,6 +194,7 @@ pub(crate) mod tests { | |||
| use sp_runtime::BuildStorage; | |||
|
|
|||
| #[test] | |||
| #[ignore] | |||
There was a problem hiding this comment.
this will require building wasm which means more time to compile and it's just for local so not that critical if this breaks
There was a problem hiding this comment.
ok I will let it compile local runtime, will revert this
There was a problem hiding this comment.
No need btw, we can just remove that test. But your call.
Minimum allowed line rate is |
Make
cargo-nextestpreferred option to run tests with fallback tocargo test.cargo-nextestmakes it easy to identify slow and leaky test and tests results are easy to read.