testutils.skip/ci: set COCKROACH_NIGHTLY_STRESS during nightlies #153135
+6
−7
Conversation
This file contains hidden or 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
Previously, `skip.Stress` returned true if either COCKROACH_STRESS or COCKROACH_NIGHTLY_STRESS env var was set. After switching to engflow, neither is set. Hence all unit tests under either `skip.Stress` or `skip.NightlyStress` were _not_ skipped. In contrast, `./dev test ... --stress` sets COCKROACH_STRESS, hence unit tests under `skip.Stress` _are_ skipped. Occassionally, a unit test's configuration might be conditional on whether it's running in nightlies. To keep things backward compatible, we set `COCKROACH_NIGHTLY_STRESS` during engflow stress nightlies and update `skip.Stress` predicate to return true iff `COCKROACH_STRESS` is set. Effectively, `skip.Stress` now denotes whether a test is executing locally under `./dev test ... --stress`, and `skip.NightlyStress` now denotes whether a test is executing remotely (via engflow) during the nightlies. Epic: none Release note: None
|
This change is |
|
@rickystewart here is our slack thread. I primarily need it for the property-based test in [1]; it could benefit from more iterations during the nightlies run. [1] #146857 |
rickystewart
reviewed
Sep 8, 2025
There was a problem hiding this comment.
This PR removes the only use of skip.NightlyStress(). So, I don't really agree with this part of the commit message:
Occassionally, a unit test's configuration might be conditional on whether it's running in nightlies.
There are no such tests that have differing behavior based on whether they're running in the nightlies.
I'm not necessarily opposed to this change, but I don't really see a reason to specifically support this if no one is using it. What about instead just deleting the NightlyStress() function?
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.
Previously,
skip.Stressreturned true if eitherCOCKROACH_STRESS or COCKROACH_NIGHTLY_STRESS env var was set. After switching to engflow, neither is set. Hence all unit tests under either
skip.Stressorskip.NightlyStresswere not skipped. In contrast,./dev test ... --stresssets COCKROACH_STRESS, hence unit tests underskip.Stressare skipped.Occassionally, a unit test's configuration might be conditional on whether it's running in nightlies.
To keep things backward compatible, we set
COCKROACH_NIGHTLY_STRESSduring engflow stressnightlies and update
skip.Stresspredicate to return true iffCOCKROACH_STRESSis set.Effectively,
skip.Stressnow denotes whether atest is executing locally under
./dev test ... --stress, andskip.NightlyStressnow denotes whether a test is executing remotely (via engflow) during the nightlies.Epic: none
Release note: None