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

ci: trade test parallelization for unconstrained gomaxprocs #12299

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Mar 15, 2022

This PR swaps t.Parallel with ci.Parallel(t), which disables running in parallel in CI. The trade-off is we can now no longer restrict GOMAXPROCS=1. That plus upping the Circle CI go machine image to large, we should get CI runs in about 15 minutes.

This is part 1 of $several for splitting up #12255; which aims to run CI on GHA.

Bad news: 419 files modified.
Good news: Only config.yaml and slow.go contain non-trivial changes.

Copy link
Member

@schmichael schmichael left a comment

Choose a reason for hiding this comment

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

Please never make me try to open that files tab again.

}
}

// Parallel runs t in parallel, unless CI is set to a true value.
Copy link
Member

Choose a reason for hiding this comment

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

Yes! I made it! You have no idea how hard Chrome had to work to let me add a comment here. After minutes of loading and many seconds to pop up this textbox after I clicked the line, my typing is limited to about 0.5 character / s.

Anyway... mind adding a comment here about why we disable parallel for CI? I assume the answer is "because we manually parallelize tests across multiple CI jobs" but it would be nice to leave a comment for future generations who find their way here (and perhaps a code change or move has obliterated this PR from git blame)

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

And thank you for your sacrifice!

@vercel vercel bot temporarily deployed to Preview – nomad March 16, 2022 13:38 Inactive
@shoenig shoenig merged commit ea91dbd into main Mar 16, 2022
@shoenig shoenig deleted the ci-parallel branch March 16, 2022 13:55
lgfa29 pushed a commit that referenced this pull request Apr 28, 2022
ci: trade test parallelization for unconstrained gomaxprocs
lgfa29 pushed a commit that referenced this pull request Apr 29, 2022
ci: trade test parallelization for unconstrained gomaxprocs
lgfa29 added a commit that referenced this pull request Apr 29, 2022
* Merge pull request #11889 from hashicorp/build-update-circle

build: upgrade circleci configuration

* Merge pull request #12299 from hashicorp/ci-parallel

ci: trade test parallelization for unconstrained gomaxprocs

* Merge pull request #12321 from hashicorp/ci-less-logging

ci: limit gotestsum to circle ci

* CI: build binaries for UI branches (#12594)

Build binaries for every code change, not just backend code
changes. This means that we'll have up-to-date compiled assets for
every commit available in CircleCI artifacts.

* Merge pull request #12736 from hashicorp/build-update-go-1.17.9

build: update golang to 1.17.9

* cgutil test: reserve only a single CPU for AddAlloc test

Reserving the entire machine's worth of CPUs for the `AddAlloc` test
triggers a condition where the empty shared CPUs defaults to the
parent CPU set on some system configurations. This was done in `main`
as part of cgroups v2 work, but we need to backport this to earlier
branches in order to use the same machines across branches.

Co-authored-by: Seth Hoenig <seth.a.hoenig@gmail.com>
Co-authored-by: Tim Gross <tgross@hashicorp.com>
lgfa29 added a commit that referenced this pull request Apr 29, 2022
* Merge pull request #11889 from hashicorp/build-update-circle

build: upgrade circleci configuration

* Merge pull request #12299 from hashicorp/ci-parallel

ci: trade test parallelization for unconstrained gomaxprocs

* Merge pull request #12321 from hashicorp/ci-less-logging

ci: limit gotestsum to circle ci

* CI: build binaries for UI branches (#12594)

Build binaries for every code change, not just backend code
changes. This means that we'll have up-to-date compiled assets for
every commit available in CircleCI artifacts.

* cgutil test: reserve only a single CPU for AddAlloc test

Reserving the entire machine's worth of CPUs for the `AddAlloc` test
triggers a condition where the empty shared CPUs defaults to the
parent CPU set on some system configurations. This was done in `main`
as part of cgroups v2 work, but we need to backport this to earlier
branches in order to use the same machines across branches.

Co-authored-by: Seth Hoenig <seth.a.hoenig@gmail.com>
Co-authored-by: Tim Gross <tgross@hashicorp.com>
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants