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

Split unit tests by time #7079

Merged
merged 5 commits into from
May 21, 2024
Merged

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented May 8, 2024

PR description

This PR reduces the time of the pre-review workflow from ~30min to <20min, splitting unit tests by time, and removing the dependency between compile and unit tests since the Gradle cache is disabled and so test tasks need to recompile anyway.
Also reduced the number of ATs runners to 10 since the workflow takes only ~10min to complete.
Next step is to see if reference tests can be split by time as well.

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@fab-10 fab-10 force-pushed the split-unit-tests-by-time branch 19 times, most recently from 2d2fe15 to 9594188 Compare May 9, 2024 15:57
@fab-10 fab-10 force-pushed the split-unit-tests-by-time branch 5 times, most recently from de93355 to 758c66f Compare May 17, 2024 15:40
Signed-off-by: Fabio Di Fabio <[email protected]>
@fab-10 fab-10 force-pushed the split-unit-tests-by-time branch 4 times, most recently from d94aa8d to 3d5314c Compare May 20, 2024 14:09
fab-10 added 2 commits May 20, 2024 17:56
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
@fab-10 fab-10 force-pushed the split-unit-tests-by-time branch from 283b2c4 to 55c52e2 Compare May 20, 2024 14:57
@fab-10 fab-10 marked this pull request as ready for review May 20, 2024 15:02
.github/workflows/pre-review.yml Outdated Show resolved Hide resolved
.github/workflows/pre-review.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

assuming this doesn't change how failures are presented?

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
@fab-10 fab-10 enabled auto-merge (squash) May 21, 2024 07:28
@fab-10
Copy link
Contributor Author

fab-10 commented May 21, 2024

assuming this doesn't change how failures are presented?

there should be no difference in that

@fab-10 fab-10 merged commit 4016447 into hyperledger:main May 21, 2024
37 checks passed
jflo pushed a commit to jflo/besu that referenced this pull request May 28, 2024
* Split unit tests by time

Signed-off-by: Fabio Di Fabio <[email protected]>

* Parallelize compile and unit tests since there is not shared cache

Signed-off-by: Fabio Di Fabio <[email protected]>

* fix

Signed-off-by: Fabio Di Fabio <[email protected]>

* Reduce ATs runnes to 10

Signed-off-by: Fabio Di Fabio <[email protected]>

* Apply suggestions from code review

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>

---------

Signed-off-by: Fabio Di Fabio <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Justin Florentine <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Jun 10, 2024
* Split unit tests by time

Signed-off-by: Fabio Di Fabio <[email protected]>

* Parallelize compile and unit tests since there is not shared cache

Signed-off-by: Fabio Di Fabio <[email protected]>

* fix

Signed-off-by: Fabio Di Fabio <[email protected]>

* Reduce ATs runnes to 10

Signed-off-by: Fabio Di Fabio <[email protected]>

* Apply suggestions from code review

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>

---------

Signed-off-by: Fabio Di Fabio <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Justin Florentine <[email protected]>
@fab-10 fab-10 deleted the split-unit-tests-by-time branch November 8, 2024 16:37
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.

2 participants