Skip to content

Remove explicit -p from CI tests#18999

Merged
kisunji merged 3 commits intomainfrom
kisunji/test
Oct 3, 2023
Merged

Remove explicit -p from CI tests#18999
kisunji merged 3 commits intomainfrom
kisunji/test

Conversation

@kisunji
Copy link
Contributor

@kisunji kisunji commented Sep 25, 2023 •
edited
Loading

Description

The -p flag defaults to GOMAXPROCS, so we have no reason to artificially limit the test parallelism when our runners have more cores. This PR averages over a minute faster than other successful CI runs.

        -p n
                the number of programs, such as build commands or
                test binaries, that can be run in parallel.
                The default is GOMAXPROCS, normally the number of CPUs available.

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added type/ci Relating to continuous integration (CI) tooling for testing or releases theme/contributing Additions and enhancements to community contributing materials labels Sep 25, 2023
@kisunji kisunji changed the title CI test Remove explicit -p from CI tests Oct 3, 2023
@kisunji kisunji added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Oct 3, 2023
@vercel
Copy link

vercel bot commented Oct 3, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
consul ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2023 1:25pm
consul-ui-staging ⬜️ Ignored (Inspect) Oct 3, 2023 1:25pm

kisunji
kisunji commented Oct 3, 2023
View reviewed changes
.github/workflows/reusable-unit-split.yml
--packages="$PACKAGE_NAMES" \
--junitfile ${{env.TEST_RESULTS}}/gotestsum-report.xml -- \
-tags="${{env.GOTAGS}}" -p 2 \
${GO_TEST_FLAGS-} \
Copy link
Contributor Author

@kisunji kisunji Oct 3, 2023

Choose a reason for hiding this comment

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

I wasn't sure what this line was doing. In L118 we already process go-test-flags so I assume this is a remnant of our CircleCI workflows

dhiaayachi
dhiaayachi approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

@kisunji kisunji merged commit 2467660 into main Oct 3, 2023
@kisunji kisunji deleted the kisunji/test branch October 3, 2023 14:11
@analogue
Copy link
Contributor

analogue commented Oct 3, 2023

Good find!

@kisunji kisunji mentioned this pull request Oct 3, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dhiaayachi dhiaayachi dhiaayachi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry theme/contributing Additions and enhancements to community contributing materials type/ci Relating to continuous integration (CI) tooling for testing or releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kisunji @analogue @dhiaayachi