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: use TESTTIMEOUT in teamcity-test #76981

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

tbg
Copy link
Member

@tbg tbg commented Feb 24, 2022

We want to be able to adjust this easily, without a round-trip through
CI.

Inspired by this run, where test hit the default 45m timeout.

Release note: None

We want to be able to adjust this easily, without a round-trip through
CI.

Inspired by [this run], where `test` hit the default 45m timeout.

[this run]: https://teamcity.cockroachdb.com/viewLog.html?buildId=4448093&buildTypeId=Cockroach_UnitTests_Test&tab=buildResultsDiv

Release note: None
@tbg tbg requested a review from a team as a code owner February 24, 2022 13:57
@tbg tbg requested a review from cucaroach February 24, 2022 13:57
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@cucaroach cucaroach left a comment

Choose a reason for hiding this comment

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

Approving this but isn't setting build properties like this in the teamcity UI as opposed to in a source controlled file a release engineering anti-pattern?

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

@tbg
Copy link
Member Author

tbg commented Feb 24, 2022

Approving this but isn't setting build properties like this in the teamcity UI as opposed to in a source controlled file a release engineering anti-pattern?

I'm not sure, that's for dev-inf to decide.

TFTR!

bors r=cucaroach

@jlinder
Copy link
Collaborator

jlinder commented Feb 24, 2022

Approving this but isn't setting build properties like this in the teamcity UI as opposed to in a source controlled file a release engineering anti-pattern?

Generally, we consider the best practice is to put as much configuration into the repo as possible. Some things can't be put in the repo (like code that switches between possible scripts in the repo depending on what is in the branch) and somethings we do not have infrastructure set up to put in the repo (like secrets). It can also be useful to allow engineers to override a configuration value set in the repo when manually starting a run.

Following that guidance, it would be preferable to set this timeout value directly in the code and possibly provide an override via the TeamCity UI. Given this script is for running the unit tests with make and make will be going away soon, I don't think changing it here is important.

@craig
Copy link
Contributor

craig bot commented Feb 24, 2022

Build succeeded:

@craig craig bot merged commit 920464f into cockroachdb:master Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants