Omit NEXTEST_NO_TESTS: fail, now that it's the default
#1707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 2fc93f7. The change to the `justfile` should be kept, but is temporarily reverted to verify that the change in `ci.yml` is no longer needed.
This was temporarily removed to verify that we no longer need to set `NEXTEST_NO_TESTS` to `fail` explicitly in `ci.yml` to get that failing behavior, now that it is implied starting in `cargo-nextest` version 0.9.85. It is now brought back, so that we do not get that error in the one place where we know there are no test cases yet do tell `nextest` to run tests.
EliahKagan
changed the title
NEXTEST_NO_TESTS: fail, now that it's impliedNEXTEST_NO_TESTS: fail, now that it's the default
|
That's a great catch! I am happy that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes
ci.ymlto no longer explicitly tellcargo nextestto treat the unexpected absence of any test cases as an error, since that is now that runner's default behavior.In 2fc93f7 (#1675), I added this to the top-level
envinci.yml, to get failures whenever acargo nextestcommand looks for tests to run but doesn't find any:gitoxide/.github/workflows/ci.yml
Line 22 in 1435193
That commit also added
--no-tests=warnin the one place where we deliberately have that happen:gitoxide/justfile
Line 189 in 1435193
Yesterday,
cargo-nextestv0.9.85 was released. This includes the planned change nextest-rs/nextest#1646 that makes failure the default behavior here. That makesNEXTEST_NO_TESTS: failsuperfluous. The change in this PR is done in two commits, to verify that it is now superfluous, but they can be squashed into one if desired.Whether we should continue to set that explicitly is subjective, which is why I'm opening a PR just for it without including anything else (so it's easy to make a decision about this, whatever the decision is). It seems to me that we shouldn't, on the grounds that it was effectively a workaround for the preferable default
cargo nextestnot automatically treating that as an error, as well as a preparation for thenextestbehavior change that has come in as of v0.9.85.