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

Run more tests concurrently in CI #1514

Merged
merged 4 commits into from
Oct 21, 2024
Merged

Conversation

jyemin
Copy link
Contributor

@jyemin jyemin commented Sep 25, 2024

Make these suites run concurrently on the main matrices

  • driver-sync
  • driver-reactive-streams
  • driver-core
  • driver-legacy

Additionally, run unit tests once per JDK rather than on every MongoDB deployment variant

  • bson
  • mongodb-crypt

JAVA-5628

Patch builds show that the slowest task drops from ~45 minutes to ~20 minutes

@jyemin jyemin self-assigned this Sep 25, 2024
- name: "test-reactive"
- name: "test-core-and-legacy"

- matrix_name: "tests-unit"
Copy link
Contributor Author

@jyemin jyemin Sep 25, 2024

Choose a reason for hiding this comment

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

Just run unit tests once on each JDK, instead of with every topology and release of MongoDB.

.evergreen/.evg.yml Show resolved Hide resolved
.evergreen/.evg.yml Outdated Show resolved Hide resolved
- func: "bootstrap mongo-orchestration"
- func: "run tests"
vars:
TESTS: 'driver-core:test driver-legacy:test'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collectively these take around 4 minutes to run on a replica set

Copy link
Member

Choose a reason for hiding this comment

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

Seems a strange combo - would it make more sense for legacy tests to be in with sync tests?

Copy link
Contributor Author

@jyemin jyemin Oct 2, 2024

Choose a reason for hiding this comment

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

Logically, yes. But mostly this is about balancing the test run times, and the driver-sync tests are already the slowest at around 12 minutes, whereas driver-core and driver-legacy together are less than 5 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to split them out to separate tasks

.evergreen/.evg.yml Outdated Show resolved Hide resolved
.evergreen/.evg.yml Outdated Show resolved Hide resolved
* Run sync tests together
* Run reactive tests together
* Run core and legacy together
* Run bson and crypt tests together

JAVA-5628
@rozza
Copy link
Member

rozza commented Oct 2, 2024

I like it. One comment about the legacy tests but it looks good 👍

@rozza
Copy link
Member

rozza commented Oct 2, 2024

So kotlin tests are duplicated here but the Scala driver tests aren't.

As we already limit the versions that we run kotlin / scala on and Scala tests multiple scala runtimes. I'd recommend removing them from the bson and reactive tests.

@jyemin jyemin requested a review from rozza October 3, 2024 11:45
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM

I have an optional suggestion - put the "test" prefix after.

eg:

- "test-sync"  -> "sync-driver-tests"      
- "test-reactive" -> "reactive-streams-driver-tests"
- "test-core" -> "core-driver-tests"
- "test-legacy" -> "legacy-driver-tests"

Ideally I'd want to use a display_name and whitespace in the evergreen UI but doesn't look like its supported 🤷

@jyemin
Copy link
Contributor Author

jyemin commented Oct 3, 2024

I have an optional suggestion - put the "test" prefix after.

It reads better with test as a suffix, but I prefer the prefix because then they all sort together

@jyemin jyemin marked this pull request as ready for review October 4, 2024 11:29
@jyemin jyemin requested a review from katcharov October 8, 2024 13:04
@jyemin
Copy link
Contributor Author

jyemin commented Oct 8, 2024

Added @katcharov to get one more set of eyes on this.

Copy link
Contributor

@katcharov katcharov left a comment

Choose a reason for hiding this comment

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

LGTM.

Nice reduction.

@jyemin jyemin merged commit 8a27730 into mongodb:main Oct 21, 2024
53 of 59 checks passed
@jyemin jyemin deleted the evergreen-experiment branch October 21, 2024 18:29
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.

3 participants