Problem/Motivation

https://www.drupal.org/pift-ci-job/2251690

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 3251891-13.patch9.04 KBandypost
#13 interdiff.txt6.13 KBandypost
#12 3251891-12.patch5.33 KBlongwave
#9 3251891.patch3.52 KBcatch
#8 3251891.patch2.91 KBcatch
#2 3251891.patch601 bytescatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Active » Needs review
FileSize
3251891.patch601 bytes

ckeditor5 fail is easy.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

> 16:41:30 Build timed out (after 110 minutes). Marking the build as aborted.

Why are the tests so much slower now?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Is CI using concurrency at all? Maybe it's time to use phpunit instead of run-tests.php?

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

mixologic and Gabor pointed me to #3038596: Mechanism that adds custom drupalci.yml configurations does not detect configs in new branches. in Slack which is root cause, Gabor made a commit that should fix it.

Status: Needs review » Needs work

The last submitted patch, 2: 3251891.patch, failed testing. View results

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

Nice, now with the real results, core compatibility tests and upgrade path tests seems to be failing, which does not seem to be surprising given this is a new major version :D

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Needs work » Needs review
FileSize
3251891.patch2.91 KB

Patch hopefully addresses UpdateScriptTest.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
FileSize
3251891.patch3.52 KB

And DependencyTest.

Using ^8 | ^9 | ^10 some places and >= 8 in others - I prefer the latter but probably doesn't hurt to have both in test data/modules anyway.

The last submitted patch, 8: 3251891.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 9: 3251891.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
3251891-12.patch5.33 KB

A couple more fixes.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff.txt6.13 KB
3251891-13.patch9.04 KB

Looks like ckeditor5 needs to remove core version string, also cleaned other places (interdiff against #9)

3bd108b0023d98b68610a56e58528371bc73f45f used to change ^8 to ^9 (patch mimics it)

$ git grep core_version_requirement|grep '.info.yml:'
core/modules/ckeditor5/tests/modules/ckeditor4to5upgrade_plugin_test/ckeditor4to5upgrade_plugin_test.info.yml:5:core_version_requirement: ^9
core/modules/ckeditor5/tests/modules/ckeditor5_definition_supporting_element/ckeditor5_definition_supporting_element.info.yml:5:core_version_requirement: ^9
core/modules/ckeditor5/tests/modules/ckeditor5_plugin_conditions_test/ckeditor5_plugin_conditions_test.info.yml:5:core_version_requirement: ^9
core/modules/ckeditor5/tests/modules/ckeditor5_plugin_elements_subset/ckeditor5_plugin_elements_subset.info.yml:5:core_version_requirement: ^9
core/modules/ckeditor5/tests/modules/ckeditor5_plugin_elements_test/ckeditor5_plugin_elements_test.info.yml:5:core_version_requirement: ^9
core/modules/ckeditor5/tests/modules/ckeditor5_test/ckeditor5_test.info.yml:5:core_version_requirement: ^9
core/modules/system/tests/modules/system_core_incompatible_semver_test/system_core_incompatible_semver_test.info.yml:6:core_version_requirement: ^7
core/modules/system/tests/modules/system_core_semver_test/system_core_semver_test.info.yml:6:core_version_requirement: ^8|^9|^10
core/modules/system/tests/modules/update_test_semver_update_n/update_test_semver_update_n.info.yml:6:core_version_requirement: ^9
core/modules/system/tests/themes/test_core_semver/test_core_semver.info.yml:6:core_version_requirement: ^8 || ^9
core/modules/system/tests/themes/test_invalid_core_semver/test_invalid_core_semver.info.yml:6:core_version_requirement: ^7
core/profiles/testing_config_overrides/testing_config_overrides.info.yml:7:core_version_requirement: '*'
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

For #12

+++ b/core/modules/system/tests/modules/system_core_semver_test/system_core_semver_test.info.yml
@@ -3,4 +3,4 @@ type: module
-core_version_requirement: ^8|^9
+core_version_requirement: ^8|^9|^10

it needs fix for 9.3 too, should be ^8 || ^9 || ^10

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Doing a test run for 9.4.x too, all these changes should be fine to backport.

longwave’s picture

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

Thanks @andypost - ignore my patch, yours is RTBC.

Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions commented

Just a small notice, isn't it easier just to make semver test modules to specify >=8 rather ^8 || ^9 || ^10. When 11.x will come this will require fixing again

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

I'd be fine with doing that here, but it also feels like making the switch everywhere is slightly out of scope since it would mean switching our coverage of version constraints slightly... so opened a follow-up #3252043: Make semver tests core major version agnostic.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 10.0.x-dev » 9.4.x-dev

We can backport this to 9.4.x so moving back.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Does it make sense to update CR https://www.drupal.org/node/3070687 to point how to deal with ^10

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 46b1d68add to 10.0.x and 09c511d26e to 9.4.x. Thanks!

longwave’s picture

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

Push failed? Neither commit exists in the repo.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Operator error :)

Thanks @longwave

  • alexpott committed 46b1d68 on 10.0.x 
    Issue #3251891 by catch, andypost, longwave: Drupal 10 branch failures

  • alexpott committed 09c511d on 9.4.x 
    Issue #3251891 by catch, andypost, longwave: Drupal 10 branch failures...

Status: Fixed » Closed (fixed)

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