Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: explicitly set SKIP_LABEL_TEST_FAILURE in compose.sh #109356

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

chrisseto
Copy link
Contributor

Previously, SKIP_LABEL_TEST_FAILURE was being set via a teamcity configuration. This change was quite opaque as the majority of CI configuration for Cockroach is stored as shell scripts within its repo. This commit follows that pattern by explicitly setting SKIP_LABEL_TEST_FAILURE in the script that runs TestComposeCompare.

Epic: None
Release note: None

Previously, `SKIP_LABEL_TEST_FAILURE` was being set via a teamcity
configuration. This change was quite opaque as the majority of CI
configuration for Cockroach is stored as shell scripts within its repo.
This commit follows that pattern by explicitly setting
`SKIP_LABEL_TEST_FAILURE` in the script that runs `TestComposeCompare`.

Epic: None
Release note: None
@chrisseto chrisseto added backport-22.2.x backport-23.1.x Flags PRs that need to be backported to 23.1 labels Aug 23, 2023
@chrisseto chrisseto requested a review from rafiss August 23, 2023 17:53
@chrisseto chrisseto requested a review from a team as a code owner August 23, 2023 17:53
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@chrisseto chrisseto self-assigned this Aug 23, 2023
@chrisseto
Copy link
Contributor Author

TFTR!

bors r =rickystewart

# failures per se. They're cases of behavioral divergences from Postgres. While
# our compatibility guarantees are not 100%, it's better to treat failures as
# information to occasionally review.
export SKIP_LABEL_TEST_FAILURE=1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the way to do it, as opposed to how the env vars are passed on line 34? i guess the difference is if the variable is used by the test runner versus the test itself?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is correct. We want the variable to be visible by bazci. Compare to line 34 which sets environment variables for the test inside the sandbox.

@chrisseto
Copy link
Contributor Author

Whoops, extra space might have tripped bors up.

bors r=rickystewart

@craig
Copy link
Contributor

craig bot commented Aug 23, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants