Improve timeout-based tests #1597
Comments
bkchr
pushed a commit
that referenced
this issue
Apr 10, 2024
* update Substrate + Polkadot + Cumulus refs * Origin -> RuntimeOrigin * weights v1.5 * update refs once again + `cargo test -p pallet-bridge-grandpa` works * started work on `cargo test -p pallet-bridge-messages` * cargo test -p pallet-bridge-relayers * cargo test -p pallet-bridge-parachains * cargo test -p millau-runtime * cargo test -p bridge-runtime-common * cargo test -p rialto-runtime * cargo test -p rialto-parachain-runtime * cargo test -p millau-bridge-node * cargo test -p rialto-bridge-node * cargo test -p rialto-parachain-collator * cargo test -p messages-relay * cargo test -p parachains-relay * cargo test -p substrate-relay * cargo test --all * cargo check -p millau-runtime --locked --features runtime-benchmarks * fix remaining test * fmt * try to allow clippy failure temporarily * Revert "try to allow clippy failure temporarily" This reverts commit d1b6593580f07e0dbeecb7ab0aa92cee98888ed3. * use min_by * Revert "use min_by" This reverts commit 33042f49ed37e8dd0505370289e17f03bf1a56ee. * Revert "Revert "use min_by"" This reverts commit 1d2204f0b14dc81e5650bb574dedb5fa78c7097d. * trigger CI * Revert "trigger CI" This reverts commit 259d91b5606743bba9d043c69f07eac6c8700ef5. * new day, new clippy warning * more clippy issues
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Not long ago, a couple of PVF host tests were gated through the
ci-only-testsfeature as they were found to be flaky when run locally. @bkchr was arguing it is a bad idea (and it really is), but reviewers concluded the solution is acceptable for the moment.Naturally, that was forgotten when building the monorepo. Right now, those tests are never run.
Of course, we could just put the feature back into the CI scripts and forget about it... Again. But the very fact that it happened has proven that the approach is really error-prone.
The tests in question are timeout-based. That is, the test is allowed to finish within some time frame, which proves that the work tested is executed with the expected parallelism (or, vice-versa, without any parallelism where it is undesirable). The concrete examples are
ensure_parallel_executionandexecute_queue_doesnt_stall_with_varying_executor_params, but that's not an exhaustive list; other timeout-based tests also present, we just have never seen them flaky and thus haven't gated them.Under this issue, I'd like to collect ideas on how to improve them and make them less dependent on host load; ideally, they shouldn't be load-dependent at all.
Please speak out if you have any ideas.
The text was updated successfully, but these errors were encountered: