Problem/Motivation

~
Now that we have 'total time' showing in jobs, it's easy to see when the test distribution is out of balance.

From @catch:

What we have now is:

  • run-tests.sh separates out #slow tests, then distributes them between each parallel job ordered by number of test methods descending.
  • It then takes all the other jobs, and disributes those between parallel jobs, also ordered by number of methods descending.
  • Number of methods includes TestWith etc. because it uses phpunit discovery courtesy of mondrake.

Steps to reproduce

Proposed resolution

Also from @catch:

The main thing this issue does:

  1. it removes #slow from tests that don't need it - this means the actually slow tests get distributed between runners better, which evens out the total time.
  2. It adds #slow to a couple of tests that do need it.

If we can find more tests in either bucket, we could even things out more. But it's easier to do this in batches because the variation across pipeline runs is so high. Which is also a potential problem for using phpunit stats to determine this.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3581111-5.test-balance-slack-thread.txt5.49 KBdww

Issue fork drupal-3581111

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch opened merge request !15183

dww’s picture

dww
we/he/they
commented
Title: Balance kernel tests » Balance Kernel and Functional tests
Status: Active » Needs review

Thanks for working on this!

The changes tab is showing tweaks to Functional, too, expanding the title.

https://git.drupalcode.org/project/drupal/-/pipelines/777661 took 9:52 total, but that doesn't say much.

The 4 Kernel jobs were:

  1. 9061850: 4 minutes 9 seconds
  2. 9061851: 4 minutes 2 seconds
  3. 9061852: 4 minutes 43 seconds
  4. 9061853: 3 minutes 39 seconds

That seems reasonably balanced. Can we move 20 seconds of tests from 3 to 4? 😅

Guess we could compare to other pipelines in other issues, but I don't want to compare apples and oranges. Maybe a recent nightly main build?

The 8 Functional jobs were:

  1. 9061842: 5 minutes 48 seconds
  2. 9061843: 5 minutes 49 seconds
  3. 9061844: 5 minutes 14 seconds
  4. 9061845: 5 minutes 13 seconds
  5. 9061846: 5 minutes 54 seconds
  6. 9061847: 5 minutes 10 seconds
  7. 9061848: 5 minutes 35 seconds
  8. 9061849: 3 minutes 33 seconds

That also seems pretty good. Don't know if we can move 30 seconds of tests from 1, 2 and 5 into 8. That'd be nearly ideal if so. 😅

smustgrave’s picture

smustgrave commented

Not sure I can review but wanted to say thanks! This is amazing and I still remember DrupalCI taking almost 2 hours.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community
Related issues: +#3040694: Use previous PHPUnit JUnit results to pack tests in bins when parallelizing, +#3390193: Add a drupalGet() method to KernelTestBase, +#3526459: [PP-1] Autodetect available CPUs in run-tests.sh for concurrency, +#3581116: Speed up DependencyTest, +#3578868: [policy, no patch] Move REST module to contrib, +#3538002: run-tests.sh - separate test allocation to bins into a WorkAllocator class, +#3523614: [CI] Collect and report deprecation statistics and details
StatusFileSize
new 3581111-5.test-balance-slack-thread.txt5.49 KB

Had a nice Slack chat just now with @catch (and @mondrake) about all this. Attaching the transcript (with permission) for posterity.

TL;DR: We don't currently have that much fine-grained control over which tests end up in which buckets, so the stuff I'm requesting/suggesting at #3 isn't really possible.

[edit] I put @catch's excellent summaries into the issue summary.

See transcript for gory details. Adding a bunch of referenced related issues for anyone else who lands here.

Given all that, I'm going to go ahead and RTBC. While this isn't perfect, it's an improvement, so let's move forward without too much more churn in here and focus on the bigger fish that might yield much more significant results.

Thanks!
-Derek

dww’s picture

dww
we/he/they
commented
Issue summary: View changes

Moving some of that to the summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Needs rebase:

error: core/tests/Drupal/KernelTests/Core/Theme/Stable9LibraryOverrideTest.php: does not exist in index
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

Rebased.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

PHPStan failure now that Stable9 test has been deleted.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

Should be happier again.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Fixed

Committed and pushed 284ea826550 to main. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !15183

  • longwave committed 284ea826 on main 
    test: #3581111 Balance Kernel and Functional tests

By: catch
By: dww

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.