Discovered in #2201783: Simplify execution logic in TestBase::run():

TestBase::run() somehow supports a magic SORT_METHODS constant on a test class that triggers an alphabetical sort of test methods prior to executing them.

That doesn't make much sense.

  1. Figure out which issue introduced it and why.
  2. Remove it.
CommentFileSizeAuthor
#4 drupal8.test-sort-methods.4.patch1.34 KBsun
#2 drupal8.test-sort-methods.2.patch1.31 KBsun
#1 drupal8.test-sort-methods.1.patch656 bytessun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Needs review
Issue tags: +Novice
StatusFileSize
new drupal8.test-sort-methods.1.patch656 bytes

Within core, that instance in TestBase::run() is the only instance of that constant name.

Note that #2201783: Simplify execution logic in TestBase::run() should land first. And after that, this patch needs a re-roll. ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal8.test-sort-methods.2.patch1.31 KB

The constant was only introduced very recently in #2106171: Write tests for simple configuration deployment scenario

However, ConfigExportImportUITest has since been rewritten to no longer rely on that construct.

Thus, attached patch additionally removes the stale/obsolete phpDoc on that test class.

Will cancel the previous test in a moment.

The last submitted patch, 1: drupal8.test-sort-methods.1.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: -Novice
StatusFileSize
new drupal8.test-sort-methods.4.patch1.34 KB

Re-rolled against HEAD.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

4: drupal8.test-sort-methods.4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: drupal8.test-sort-methods.4.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review

4: drupal8.test-sort-methods.4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: drupal8.test-sort-methods.4.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review

4: drupal8.test-sort-methods.4.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

4: drupal8.test-sort-methods.4.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Hm. If that test is still passing without this (which it appears it is) then this is good to go. But I know that code was added explicitly so that you could test an export before testing an import, since the second test relied on the former. I guess we can always re-introduce this again if there are problems expanding those tests.

Committed and pushed to 8.x. Thanks!

  • Commit d80bd85 on 8.x by webchick: 
    Issue #2202377 by sun: Remove Test::SORT_METHODS constant opt-in support...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Test methods should *not* depend on each other. They are supposed to work in isolation. That should simply be one test method split into two helper methods, if it's not already.

Status: Fixed » Closed (fixed)

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