build: rebalance unit test shards to reduce CI critical path (~29m --> ~23m) and use fewer workers (16 --> 10) by feanil · Pull Request #38287 · openedx/openedx-platform

feanil · 2026-04-04T04:44:26Z

Summary

Rebalances the unit test shard configuration to reduce the CI critical path from ~29 minutes down to ~23 minutes, measured across 3 consistent runs.

What changed

Replaced 16 uneven shards with 10 balanced shards targeting ~19–23 min each
New shard layout: 5 LMS shards, 2 shared-with-LMS shards, 1 shared-with-CMS shard, and 2 CMS shards
- `cms-1`: small CMS apps (api, cms_user_tasks, course_creators, envs, lib, etc.) — ~5–6 min
- `cms-2`: `contentstore/` only — ~19–20 min (split out to avoid OOM on runners when combined with other CMS apps)
Also includes test isolation fixes (also in fix: test isolation fixes for certificates and openedx-events test mixins #38347 for independent merging):
- Moved `CourseFactory()` calls from `setUp` to `setUpClass` in three certificate test classes to avoid exhausting MongoDB connections across test methods
- Updated to openedx-events 11.1.1 which adds `tearDownClass` to `OpenEdxEventsTestMixin`, preventing events from being left globally disabled between test classes

Timing data — 3 runs on the new config (all passed)

Shard	Run 1	Run 2	Run 3	Avg
`shared-with-cms-1`	22m48s	22m33s	23m09s	~22m50s
`shared-with-lms-1`	21m57s	22m49s	23m01s	~22m36s
`shared-with-lms-2`	20m06s	20m54s	22m05s	~21m02s
`lms-4`	22m28s	21m07s	21m27s	~21m41s
`lms-1`	21m18s	19m23s	22m25s	~21m02s
`lms-5`	19m50s	21m11s	19m22s	~20m08s
`cms-2`	19m31s	18m13s	20m19s	~19m21s
`lms-2`	19m16s	19m19s	18m43s	~19m06s
`lms-3`	18m43s	18m48s	18m52s	~18m48s
`cms-1`	5m40s	5m27s	5m32s	~5m33s
Critical path	~23m	~23m	~23m	~23m

For comparison, the old config's critical path (visible in the #38347 run on unmodified master) was ~29m on lms-4.

feanil · 2026-04-06T14:05:31Z

The fix needs to be made upstream: openedx/openedx-events#559 waiting for that to be merged and released before coming back to this PR.

Run pytest with extra reporting enabled to generate files with per-test durations. The file is uploaded as a CI artifact so timing data can be downloaded and used to drive optimal shard rebalancing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Redistribute test paths across 9 shards (down from 16) using a greedy bin-packing optimiser driven by real per-test timing data from pytest-reportlog. Predicted critical path: ~18.7m (down from ~29m). Key changes: - Rename shard groups to reflect semantic meaning: lms-*, shared-with-lms-*, shared-with-cms-*, cms-* (openedx/common/xmodule paths explicitly separated from lms-only and cms-only paths) - Split lms/djangoapps/discussion/ into its 4 subdirectories so the heavy rest_api/ shard (15.7m) can be distributed across bins independently - Remove outdated comment referencing unit-tests-gh-hosted.yml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ubclasses Three test classes in the certificates app were calling CourseFactory() in setUp() despite extending SharedModuleStoreTestCase. Unlike ModuleStoreTestCase, SharedModuleStoreTestCase shares a single modulestore across all tests in the class and only closes MongoDB connections at tearDownClass. Calling CourseFactory() in setUp() created a new MongoDB course (and opened connections) for every test method without releasing them, causing connection accumulation across the full test run. Affected classes: - CertificateFiltersTest (test_filters.py) - CertificateInvalidationTest (test_models.py) - CertificateAllowlistTest (test_models.py) In each case the course is only read by test methods (test data such as users, enrollments and certificates is written via Django ORM and rolled back between tests), so sharing a single course across the class is correct. See: https://github.com/openedx/openedx-platform/blob/master/xmodule/modulestore/tests/django_utils.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…vents.testing openedx_events/tests/utils.py was moved to openedx_events/testing.py in openedx/openedx-events#559 so the test utilities are included in the installed package (setup.py excludes the tests/ subpackage from the wheel). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

When OpenEdxEventsTestMixin was listed after a TestCase subclass (e.g. Foo(SharedModuleStoreTestCase, OpenEdxEventsTestMixin)), it landed after unittest.case.TestCase in the MRO. Since unittest.case.TestCase.setUpClass and tearDownClass do not call super(), the mixin's lifecycle methods never ran. The workaround was to manually call cls.start_events_isolation() in each class's setUpClass, but there was no corresponding tearDownClass to restore event state, causing events disabled by one test class to leak into subsequent classes in the same process. Fix by placing OpenEdxEventsTestMixin first in the base class list so it appears before unittest.case.TestCase in the MRO. This lets setUpClass and tearDownClass run automatically through the cooperative super() chain, removing the need for manual start_events_isolation() calls. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

This mixin is already included via one of the other mixins on this test class so including it again was messing with the MRO for the test classes.

contentstore/ is large enough that the cms-1 runner was being killed mid-run in CI (OOM or runner-level timeout). Splitting it into its own shard keeps each job under the ~20-25 min target. No changes needed to gha_unit_tests_collector.py — it already classifies any shard whose first path starts with "cms/" as a CMS shard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kdmccormick

Nice

kdmccormick · 2026-04-13T13:53:03Z

.github/workflows/unit-test-shards.json

    ]
  },
-  "openedx-1-with-cms": {
+  "shared-with-cms-1": {


It was nice for debugging that for each X-with-lms shard there was a corresponding X-with-cms, especially for those tests which would pass in one system and fail in the other. Would you be willing to change it so that we have parallel shared-with-lms-[1,2] and shared-with-cms-[1,2] shards? It would only add one additional shard and I don't think it'd increase the critical test time.

If not, then could you simplify shared-with-cms-1 definition into just the paths xmodule/, common/, and openedx/?

I split them for convenience for now. I didn't want to collapse the tests because that will make it harder to re-balance them in the future since it would require more lookups to do the rebalancing.

Did you catch this feedback?

It was nice for debugging that for each X-with-lms shard there was a corresponding X-with-cms, especially for those tests which would pass in one system and fail in the other. Would you be willing to change it so that we have parallel shared-with-lms-[1,2] and shared-with-cms-[1,2] shards? It would only add one additional shard and I don't think it'd increase the critical test time.

Nevermind, just saw your commit

TIL, that apparently we don't run all of the openedx apps under CMS, just some of them and if we try to run all of them there are issues:

https://github.com/openedx/openedx-platform/actions/runs/24354051533/job/71116066612?pr=38287

https://github.com/openedx/openedx-platform/actions/runs/24354051533/job/71116066418?pr=38287

These folders are run under LMS but not CMS:

openedx/core/djangoapps/course_live/ openedx/core/djangoapps/notifications/ openedx/core/djangolib/ openedx/core/tests/ openedx/features/ openedx/testing/

What do you think about landing this as is? I think it could be further improved and there's more to investigate but I don't want this to be blocked on existing issues.

Wow that's crazy

Yeah, in that case, totally OK with there being just one CMS shard

I wrote a followup issue, do you mind linking it here with a TODO comment? #38355

Otherwise, LGTM ✅

.github/workflows/unit-tests.yml

Per #38287 (comment) Having the shared-with... tests correspond between the LMS and CMS makes it easier to spot tastes that are failing in one context but not the other more quickly. Since neither of these is the longest run, we pay a bit more in overhead for this but it's still an improvement over what we had.

kdmccormick

🚀

feanil · 2026-04-13T19:50:12Z

@kdmccormick take a look at my latest comment in the conversation above, the extra commit revealed issues, I could still split shared-cms to 2 shards removing those highlighted test files but not sure if it's valuable. What do you think?

feanil force-pushed the feanil/collect-test-timings branch 2 times, most recently from afef03c to c17633d Compare April 4, 2026 18:45

feanil force-pushed the feanil/collect-test-timings branch 2 times, most recently from 676809c to a89e30c Compare April 10, 2026 21:11

feanil mentioned this pull request Apr 10, 2026

fix: test isolation fixes for certificates and openedx-events test mixins #38347

Open

feanil force-pushed the feanil/collect-test-timings branch from a89e30c to 19c5643 Compare April 11, 2026 02:20

feanil and others added 8 commits April 11, 2026 11:52

fix: Don't include OpenEdxEventsTestMixin twice.

29f2aa7

This mixin is already included via one of the other mixins on this test class so including it again was messing with the MRO for the test classes.

chore: Upgrade openedx-events to the latest version.

e95bbce

feanil force-pushed the feanil/collect-test-timings branch from 19c5643 to 1422307 Compare April 11, 2026 15:53

feanil changed the title ~~build: collect per-test timing data via --report-log~~ build: rebalance unit test shards to reduce CI critical path (~29m → ~23m) Apr 11, 2026

feanil marked this pull request as ready for review April 11, 2026 17:07

feanil requested review from a team, farhan, irtazaakram and salman2013 as code owners April 11, 2026 17:07

feanil requested a review from a team April 11, 2026 17:07

feanil changed the title ~~build: rebalance unit test shards to reduce CI critical path (~29m → ~23m)~~ build: rebalance unit test shards to reduce CI critical path (~29m → ~23m) and use fewer workers (16 -> 10) Apr 11, 2026

feanil changed the title ~~build: rebalance unit test shards to reduce CI critical path (~29m → ~23m) and use fewer workers (16 -> 10)~~ build: rebalance unit test shards to reduce CI critical path (~29m --> ~23m) and use fewer workers (16 --> 10) Apr 11, 2026

kdmccormick reviewed Apr 13, 2026

View reviewed changes

feanil requested a review from kdmccormick April 13, 2026 16:17

kdmccormick approved these changes Apr 13, 2026

View reviewed changes

feanil force-pushed the feanil/collect-test-timings branch from dcbee9b to 1422307 Compare April 13, 2026 19:44

feanil requested a review from kdmccormick April 13, 2026 19:50

kdmccormick mentioned this pull request Apr 13, 2026

Run tests on all shared folders using CMS settings #38355

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: rebalance unit test shards to reduce CI critical path (~29m --> ~23m) and use fewer workers (16 --> 10)#38287

build: rebalance unit test shards to reduce CI critical path (~29m --> ~23m) and use fewer workers (16 --> 10)#38287
feanil wants to merge 8 commits intomasterfrom
feanil/collect-test-timings

feanil commented Apr 4, 2026 •

edited

Loading

Uh oh!

feanil commented Apr 6, 2026

Uh oh!

kdmccormick left a comment

Uh oh!

kdmccormick Apr 13, 2026

Uh oh!

feanil Apr 13, 2026

Uh oh!

kdmccormick Apr 13, 2026

Uh oh!

kdmccormick Apr 13, 2026

Uh oh!

feanil Apr 13, 2026

Uh oh!

kdmccormick Apr 13, 2026

Uh oh!

Uh oh!

kdmccormick left a comment

Uh oh!

feanil commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

feanil commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Timing data — 3 runs on the new config (all passed)

Uh oh!

feanil commented Apr 6, 2026

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

kdmccormick Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

feanil Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

feanil Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

kdmccormick Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

feanil commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feanil commented Apr 4, 2026 •

edited

Loading