- Issue created by @catch
- Status changed to Needs review
12 months ago 3:57pm 25 September 2023 - ๐ฌ๐งUnited Kingdom catch
Opened an MR.
UpdateSemverTestBase had a method with a data provider as well as multiple one off tests.
The one-off tests move to one trait.
The data provider tests move to another trait.
UpdateSemverContribTest is split to two new tests that each implement one trait each.
UpdateSemverCoreTest also does this, but is additionally gets UpdateSemverCoreSecurityCoverageTest which also uses a data provider, and the original test class is left with its one-off methods.
This means we split two mammoth tests into a total of six. They still each contain multiple test methods and take minutes to run but with ๐ Distribute @group #slow tests between test runners and mark more tests RTBC that might be within threshold.
The threshold is defined as - if all other tests run by a runner take x minutes to run, the slowest test should finish at the same time as, or before, other tests, and not artificially extend the run beyond this - assuming parallel tests and concurrency this is probably going to be between 3-5 minutes and we can tweak parallel tests and concurrency to match what a realistic finishing time looks like.
- last update
12 months ago Custom Commands Failed - @catch opened merge request.
- last update
12 months ago Custom Commands Failed - last update
12 months ago 30,208 pass - ๐ฌ๐งUnited Kingdom catch
If you look at this job https://git.drupalcode.org/project/drupal/-/jobs/114753 (from the omnibus 'speed up gitlab ci runs' issue) you can see these tests starting first and finishing about half way through the job, which is ideal.
- last update
12 months ago 30,208 pass - First commit to issue fork.
- Open on Drupal.org โEnvironment: PHP 8.2 & MySQL 8last update
12 months ago Not currently mergeable. - last update
12 months ago 30,363 pass - ๐ฌ๐งUnited Kingdom catch
Those changes look good, can't RTBC here because I started it off.
- ๐ญ๐บHungary Gรกbor Hojtsy Hungary
Clever to split the tests like this to allow parallelization. I think the traits look good and the different tests are now nicely compartmentalized. I noted that
UpdateSemverContribTestBase
lost its class phpdoc while being renamed.UpdateSemverCoreTestBase
class is newly added and did not get phpdoc. Are those not required anymore? :)That's the only thing I found.
- last update
11 months ago 30,361 pass - Status changed to RTBC
11 months ago 2:55pm 29 September 2023 - ๐ญ๐บHungary Gรกbor Hojtsy Hungary
Looks all good to me. I think the split of traits and base classes makes sense and its good to see how it speeds up tests.
- ๐บ๐ธUnited States dww
I had some concerns on the comments when I looked last night. Iโll have to check again with the latest commits.
- Assigned to dww
- Status changed to Needs work
11 months ago 9:29pm 29 September 2023 - ๐บ๐ธUnited States dww
Itโs better, but some of the new stuff has vague / duplicate comments, or nothing to help make sense of whatโs what. I need to get my laptop out and make some concrete suggestions (or just push a commit). First, lunch. ๐
- last update
11 months ago 30,360 pass - Issue was unassigned.
- Status changed to Needs review
11 months ago 10:46pm 29 September 2023 - ๐บ๐ธUnited States dww
It was easier to push a commit to cleanup all the file comments. It's not totally perfect, but it's a lot less copy/paste and a lot more accurate. ๐ Also rebased to the very latest 11.x.
I'll note that
UpdateSemverCoreTest
is no longer really testing anything about semver for core as such. It's all the edge-case functional tests for various aspects of the Update Manager UI that we decided to add coverage for. Core uses semver, so this test uses all the semver* plumbing, but it's not really a test of "core semver" anymore. However, I don't think it's worth renaming it at this point. I left a slightly more verbose file comment at the top. Feedback welcome.Thanks, y'all!
-Derek - ๐บ๐ธUnited States dww
p.s. Normally I wouldn't hold up important test performance issues like this over doc cleanups. But Update Manager tests are notoriously confusing and hard to follow, and I think it's crucial to try to make them as well documented as possible to make it feasible for other people to contribute to them over time...
Thanks/sorry,
-Derek - Status changed to RTBC
11 months ago 6:04am 30 September 2023 - ๐ญ๐บHungary Gรกbor Hojtsy Hungary
I reviewed your doc updates at https://git.drupalcode.org/project/drupal/-/merge_requests/4872/diffs?co... and they are good improvements IMHO.
47:45 44:02 Running- last update
11 months ago 30,362 pass - Status changed to Fixed
11 months ago 5:30pm 2 October 2023 -
lauriii โ
committed 8b1f3553 on 11.x
Issue #3389510 by catch, tedbow, dww, Gรกbor Hojtsy: Split up update...
-
lauriii โ
committed 8b1f3553 on 11.x
Automatically closed - issue fixed for 2 weeks with no activity.