ci: prevent miri fail by allowing nextest to pass on no-tests condition#1978
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 963791c931
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| PACKAGES="$PACKAGES -p $crate" | ||
| done | ||
| cargo nextest run $PACKAGES --profile ci --test-threads=1 --verbose -E 'test(tracing_integration_tests::)' | ||
| cargo nextest run $PACKAGES --profile ci --test-threads=1 --verbose --no-tests=pass -E 'test(tracing_integration_tests::)' |
There was a problem hiding this comment.
Keep tracing integration runs failing on empty filters
In the full-workspace Linux path, this step exists specifically to run tracing_integration_tests::; adding --no-tests=pass makes it succeed if that filter stops matching anything (for example after a test module is renamed, cfg-gated out, or removed). The PR-specific branch already skips only when no tracing crate is selected, so when this command is reached an empty test set should remain a CI failure rather than silently dropping all tracing integration coverage.
Useful? React with 👍 / 👎.
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 963791c | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1978 +/- ##
==========================================
- Coverage 72.65% 72.62% -0.03%
==========================================
Files 450 450
Lines 74021 74021
==========================================
- Hits 53781 53761 -20
- Misses 20240 20260 +20
🚀 New features to boost your workflow:
|
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
| cache-targets: true # cache build artifacts | ||
| cache-bin: true # cache the ~/.cargo/bin directory | ||
| - run: MIRIFLAGS="-Zmiri-disable-isolation" cargo miri nextest run ${{ needs.setup.outputs.crates }} --partition count:${{ matrix.partition }}/5 | ||
| - run: MIRIFLAGS="-Zmiri-disable-isolation" cargo miri nextest run ${{ needs.setup.outputs.crates }} --partition count:${{ matrix.partition }}/5 --no-tests=pass |
There was a problem hiding this comment.
This might just be concern trolling, but is it possible in the future we incorrectly filter out test and not know about it? I guess we'd catch it if tests start breaking on the MergeQueue?
There was a problem hiding this comment.
According to nextest documentation that option only applies when not a single test is executed, by default nextest fails if the suite is empty. The worst thing can happen is that a crate with no tests will be merged but I guess that will be detected over the PR review process. In any case I think we can tweak the code coverage workflow to emit a warning when a crate has no tests?
On the other hand as you said, the tests for the workspace will be executed for push events so they will be run on the merge queue.
What does this PR do?
CI: nextest hardening —
.github/workflows/{miri,test,coverage}.yml--no-tests=passto every dynamic-scope nextest invocation(9 total). Prevents exit-code-4 "no tests to run" failures when the
affected-crates list contains a test-less crate.
--exclude builderand one jq filterselect(. != "builder")that all served the same purpose as thenew flag.
Motivation
Tests can fail if no tests are run by nextest. This will allow future utility crates to be compiled without failing.