Problem/Motivation

After all child issues of #3414655: [META] Convert Functional tests classes which make no HTTP requests into Kernel tests are committed we should reevaluate the break-up of all the testsuites on GitLab.

Already now, after "only" six of them are committed the job "PHPUnit Kernel 1/2" is the slowest and is taking ~30 seconds more than the next slowest job ("PHPUnit Functional Javascript 1/2")

Steps to reproduce

Proposed resolution

- Reshuffle tests/break-ups so they all take roughly the same amount of time

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3415004

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

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Postponed

Postponed on all of the child issues of #3414655: [META] Convert Functional tests classes which make no HTTP requests into Kernel tests being committed.

catch’s picture

catch
he/him
Primary language English
commented

Yes these were already finishing just around the same time as other jobs so not surprising that moving tests over could push them over the top.

Couple of options, could do one or both:

1. Change parallel: 2 to parallel: 3
2. Move them up in the YAML before functional tests (this puts them in the queue earlier, often makes a few seconds difference).

If we do #1, we also need to check for 'overhanging' tests (slow kernel tests that finish last in a job) and add those to @group #slow so they get distributed across the start of each test job instead.

It would be nice to be able to also reduce the parallel of functional tests to match, but probably need to shift quite a few more tests before that can be done without having an effect.

spokje’s picture

spokje
Primary language Dutch
commented

With already ~30 seconds "lead" (or rather trailing) and seeing the amount of NR/RTBC issue that will convert more Functional => Kernel test, I think only option #2 won't cut it.

In fact, but that's IMHO, I don't see much gain in it even when combined with #1.

I would go all-in on #1.

But let's first get all the children home/committed so we can assess the full "damage" done :)

spokje’s picture

spokje
Primary language Dutch
commented

Just for giggles/n =1/yada yada

These are the duration times of https://git.drupalcode.org/issue/drupal-3245997/-/pipelines/76305 which ran about 3 hours before the commit of the first of the "Convert BlahTest into a Kernel Test" was committed:

EDIT: Updated times to "Test run duration"

PHPUnit Kernel 1/2 - Test run duration: 4 min 5 sec
PHPUnit Kernel 2/2 - Test run duration: 4 min 2 sec
PHPUnit Functional 1/7 - Test run duration: 3 min 55 sec
PHPUnit Functional 2/7 - Test run duration: 3 min 29 sec
PHPUnit Functional 3/7 - Test run duration: 3 min 7 sec
PHPUnit Functional 4/7 - Test run duration: 3 min 11 sec
PHPUnit Functional 5/7 - Test run duration: 4 min
PHPUnit Functional 6/7 - Test run duration: 3 min 50 sec
PHPUnit Functional 7/7 - Test run duration: 4 min 9 sec
catch’s picture

catch
he/him
Primary language English
commented

It's usually safer to go by 'Test run duration' which you can get from ctrl-f in the logs. e.g. https://git.drupalcode.org/issue/drupal-3245997/-/jobs/622282 4 min 5 sec.

There's still a lot of variation, but it excludes things like 'waiting for pod to be ready' which is even more variable.

spokje’s picture

spokje
Primary language Dutch
commented

Well that's slightly annoying, I expected since there's also a Queued time, that all the overhead would be deducted.

Anyway, thanks for the insight, updated the times.

catch’s picture

catch
he/him
Primary language English
commented

The queue time is while the job is queued, but once the job starts, it still needs to get a pod to actually run the tests on and that's not included in the queue time - it's due to the interaction between gitlab and kubernetes (what I typed is about as much as I understand - but it's something along those lines).

spokje’s picture

spokje
Primary language Dutch
commented
Status: Postponed » Active
Related issues: +#3414655: [META] Convert Functional tests classes which make no HTTP requests into Kernel tests

Even if not all children of #3414655: [META] Convert Functional tests classes which make no HTTP requests into Kernel tests are in (there's one or two incoming later), I think it's time to unpostpone this issue.

By now both the Kernel test job are taking about ~1 minute longer than the Functional test jobs.

I think it's time to split the two Kernel jobs into three?

@catch I'll happily leave this in your capable hands if you have time and interest in this>

spokje’s picture

spokje
Primary language Dutch
commented
Title: [PP-1] Reevaluate the break-up of the various testsuites on GitLab » Reevaluate the break-up of the various testsuites on GitLab
catch’s picture

catch
he/him
Primary language English
commented

I've actually been messing around in https://git.drupalcode.org/project/drupal/-/merge_requests/6271 already, but let's get a 'clean' MR on here.

catch opened merge request !6325

catch’s picture

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

This successfully lowers the long runtime of the two kernel jobs, but it does it a bit unevenly, so we may have a few more @group #slow to add to even things out between the three jobs.

spokje’s picture

spokje
Primary language Dutch
commented

This successfully lowers the long runtime of the two kernel jobs, but it does it a bit unevenly,

Agreed.

so we may have a few more @group #slow to add to even things out between the three jobs.

Not sure if you want to get this in first and do the above in a follow-up (in which case I would happily RTBC this), or want to do that in here as well?

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs followup

@catch can we open follow ups for the slow groups. Would be easier to find in a search later if we ever needed to.

catch’s picture

catch
he/him
Primary language English
commented

I think it's fine to do that in a follow-up yeah, it's a bit neverending unfortunately because as you find some, that exposes more. Eventually we can try to order tests by how long they take (descending) rather than the manual @group #slow.

  • larowlan committed a16f3fc8 on 10.2.x 
    Issue #3415004 by catch, Spokje: Reevaluate the break-up of the various...

  • larowlan committed e94e1d77 on 11.x 
    Issue #3415004 by catch, Spokje: Reevaluate the break-up of the various...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 11.x and backported to 10.2.x - thanks folks

larowlan closed merge request !6325

Status: Fixed » Closed (fixed)

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

smustgrave’s picture

smustgrave commented
Issue tags: -Needs followup

Follow up added here #3421489: Add @slow group to newly split kernel jobs