ci: parallelize server tests 4-way and speed up the install phase - #3587
ci: parallelize server tests 4-way and speed up the install phase#3587RitvikSardana wants to merge 7 commits into
Conversation
b3b3897 to
25f1ddd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3587 +/- ##
===========================================
+ Coverage 68.00% 68.61% +0.61%
===========================================
Files 137 140 +3
Lines 8895 9189 +294
===========================================
+ Hits 6049 6305 +256
- Misses 2846 2884 +38 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Drop the ERPNext isolation: revert the WITH_ERPNEXT guards in install.sh and the skipUnless test guards, and remove the separate integrations workflow. ERPNext installs and its integration tests run on every PR again. Bump the split from 2 to 4 containers so the test phase absorbs the extra suite.
Port the modern frappe/frappe CI optimizations that attack the install floor (now the bottleneck after the 4-way split): - MariaDB runs in tmpfs (data dir in RAM) so site reinstall and the DB-bound test setup stop hitting disk. - Disable per-commit durability (innodb_flush_log_at_trx_commit=0, sync_binlog=0) on the ephemeral CI database. - Install frappe-bench via uv (cached setup-uv) instead of pip. - Overlap independent install steps: apt system deps run while the bench tool installs; the ERPNext clone runs during bench init. - Shallow-clone ERPNext (--depth 1) instead of a full-history clone.
Helpdesk had no codecov.yml, so codecov's strict defaults failed the project status on any coverage drop (even 0.01%). Mirror the frappe and erpnext config: allow the total to dip up to 1% before failing, and gate new code at 85% patch coverage (PR-only, ignored when tests fail).
|
Tick the box to add this pull request to the merge queue (same as
|
Confidence Score: 5/5CI-only changes with no impact on application code; safe to merge once branch protection rules are updated to match the new matrix job names. All changes are confined to CI infrastructure. Background-process error handling is correct, the MariaDB tmpfs is scoped per job, and the Files Needing Attention: No files require special attention beyond the branch-protection/mergify rule rename noted in the PR description. Reviews (3): Last reviewed commit: "ci: add codecov config to tolerate small..." | Re-trigger Greptile |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| container: [1, 2, 3, 4] |
There was a problem hiding this comment.
Matrix is 4-way, not 2-way as described. The PR description says "2-way test split" with
--total-builds 2, but container: [1, 2, 3, 4] with --total-builds 4 spins up 4 parallel jobs. This quadruples CI concurrency/cost and changes the expected check names (Python Unit Tests (1)–(4)), not (1)/(2) as noted in the PR description.
| @@ -19,6 +39,10 @@ cp -r "${GITHUB_WORKSPACE}/.github/helpers/site_config_mariadb.json" ~/frappe-be | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL character_set_server = 'utf8mb4'" | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'" | |||
|
|
|||
| # Ephemeral CI database: trade crash-safety for far fewer fsyncs. | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL innodb_flush_log_at_trx_commit = 0" | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL sync_binlog = 0" | |||
|
|
|||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "CREATE USER 'test_frappe'@'localhost' IDENTIFIED BY 'test_frappe'" | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "CREATE DATABASE test_frappe" | |||
| mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "GRANT ALL PRIVILEGES ON \`test_frappe\`.* TO 'test_frappe'@'localhost'" | |||
| @@ -40,7 +64,13 @@ sed -i 's/schedule:/# schedule:/g' Procfile | |||
| sed -i 's/socketio:/# socketio:/g' Procfile | |||
| sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile | |||
|
|
|||
| bench get-app erpnext --branch "develop" | |||
| if ! wait "$erpnext_pid"; then | |||
| echo "ERPNext clone failed:" | |||
| cat ~/erpnext_clone.log | |||
| exit 1 | |||
| fi | |||
There was a problem hiding this comment.
WITH_ERPNEXT guard described but not implemented. The PR description promises a WITH_ERPNEXT: no env-var path that skips ERPNext in the main job, but install.sh always clones and installs ERPNext unconditionally — there is no conditional branch on WITH_ERPNEXT anywhere in the diff. The described ERPNext isolation workflow also does not appear in the changed files. The main speed gain from decoupling ERPNext is not actually realized here.
|
@greptileai rereview |
1 similar comment
|
@greptileai rereview |
What
Cut Server CI wall-clock (was ~8m40) with two independent optimizations, plus a codecov fix.
1. Parallelize the test phase (4-way)
Python Unit Testsis now a 4-job matrix using Frappe's built-in split runner (run-parallel-tests --total-builds 4 --build-number N). ERPNext stays installed and its integration tests run on every PR as part of the normal suite. No test isolation, no path-gating.2. Speed up the install phase
The install phase is now the floor, so this ports the modern
frappe/frappesetup-action techniques:innodb_flush_log_at_trx_commit=0,sync_binlog=0) to skip per-commit fsyncs.uv tool install frappe-bench(with cachedsetup-uv) instead ofpip.bench init.--depth 1) instead of a full-history clone.3. Codecov
Added
codecov.yml(helpdesk had none, so strict defaults failed the project status on any coverage drop). Mirrors frappe/erpnext: project status tolerates a 1% dip; new code gated at 85% patch coverage (PR-only, ignored when tests fail).Note for required checks
The check name changes from
Python Unit TeststoPython Unit Tests (1)…(4). Branch protection / mergify rules referencing the old name need updating.How to test
CI on this PR: the four matrix jobs should pass with roughly a quarter of the tests each, ERPNext present in the install log, and a faster install phase.