scripts: improve trigger-pr-roachtest.sh#171008
Merged
trunk-io[bot] merged 2 commits intoMay 27, 2026
Merged
Conversation
Clarify that PR numbers are recommended over branch names, as branch names only work for branches pushed directly to cockroachdb/cockroach, not personal forks. When triggering roachtests from fork branches, TeamCity fails with "branch does not correspond to any branch monitored by the build VCS roots" and marks the test as ignored. Using the PR number works in all cases, as TeamCity can resolve PR branches regardless of which repository they originate from. Release note: None
The nightly TeamCity script (perturbation_nightly_metamorphic_impl.sh
and roachtest_nightly_impl.sh) invoke roachtest with
--use-spot="${USE_SPOT:-auto}". The "auto" mode uses spot VMs for the
first attempt, falling back to non-spot if the test fails due to
preemption. This works well for the nightly run (COUNT>1) where the
fallback recovers preempted tests.
PR-triggered runs typically use COUNT=1, so there is no second attempt
and every spot preemption surfaces as a test failure. The triggered
build's report shows "VMs preempted during the test run" rather than
real test results, making it impossible to evaluate the change.
Default USE_SPOT to "never" in the trigger script so PR-triggered runs
get real (non-spot) VMs by default. Callers who want the legacy
behavior can still set USE_SPOT=auto explicitly.
Release note: None
Contributor
|
😎 Merged successfully - details. |
Member
|
This change is |
arulajmani
approved these changes
May 27, 2026
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Two small improvements to
scripts/trigger-pr-roachtest.sh:Better help text: Clarify that PR numbers are recommended over branch names. Branch names only work for branches pushed directly to
cockroachdb/cockroach, not personal forks. Triggering by branch name from a fork results in TeamCity reporting "branch does not correspond to any branch monitored by the build VCS roots" and the test being ignored.Default
USE_SPOT=never: The nightly TeamCity script (perturbation_nightly_metamorphic_impl.shandroachtest_nightly_impl.sh) invokes roachtest with--use-spot="${USE_SPOT:-auto}". Theautomode uses spot VMs for the first attempt, falling back to non-spot if the test fails due to preemption. This works well for the nightly run (COUNT>1) where the fallback recovers preempted tests.PR-triggered runs typically use COUNT=1, so there is no second attempt and every spot preemption surfaces as a test failure. The triggered build's report shows "VMs preempted during the test run" rather than real test results, making it impossible to evaluate the change. Default
USE_SPOTto"never"so PR-triggered runs get real (non-spot) VMs. Callers who want the legacy behavior can still setUSE_SPOT=autoexplicitly.Motivation
Both findings came out of trying to validate #170724 (re-enabling perturbation/full tests). The first two attempts saw 5/6 tests fail with VM preemptions, masking the actual test behavior. Once
USE_SPOT=neverwas set, 5/6 tests passed cleanly. See the test results table in #170724 (comment) for the before/after.Epic: none
Release note: None