The patch attached adds two new options to run-tests.sh

The following command will run the StateSystemUpgradePathTest 100 times and if a failure occurs it will exit immediately.

./core/scripts/run-tests.sh --die-on-fail --repeat 100 --class "Drupal\system\Tests\Upgrade\StateSystemUpgradePathTest"

You can then change your settings.php to use simpletest db suffix to debug the test failure.

CommentFileSizeAuthor
#18 1825592-18.patch4.57 KBpietmarcus
#15 1825592.drupal8.test-random-fail.15.patch4.72 KBalexpott
#15 14-15-interdiff.txt1.59 KBalexpott
#14 drupal8.test-random-fail.14.patch3.33 KBsun
#13 drupal8.test-random-fail.13.patch2.38 KBsun
#9 3-9-interdiff.txt1.02 KBalexpott
#9 1825592-9.simpletest-die-on-fail-and-repeat.patch5.38 KBalexpott
#4 1825592-3.simpletest-die-on-fail-and-repeat.patch5.05 KBalexpott
#1 1825592-1.simpletest-die-on-fail-and-repeat.patch5.08 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
1825592-1.simpletest-die-on-fail-and-repeat.patch5.08 KB

And now adding patch :)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

+1, this is really helpful to track these nasty random failures down.

Maybe we can move the exit a bit down after checking and printing the error log? Not sure if it's necessary, as we likely already know what the error is when using this, just not under which circumstances it happens.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

Rerolled...

The reason I put the exit where I did is because the simpletest_script_cleanup() routine removes the db and files as well as printing the log and we want to keep these.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
1825592-3.simpletest-die-on-fail-and-repeat.patch5.05 KB

And here by the patch... Cheers @swentel

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Reviewed & tested by the community

Alright, let's get this in, this is so freaking handy

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Category: feature » task
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

That seems incredibly handy! So handy, that I'd love to see an option for "Die on fail" exposed at admin/config/development/testing/settings, too. :)

Switching to a task rather than a feature so it doesn't get gummed up on thresholds.

Seems like at least the extra params (possibly the UI change too; it's not like this is a very user-facing form) should be eligible for backport.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

So handy, that I'd love to see an option for "Die on fail" exposed at admin/config/development/testing/settings, too. :)

I think this is out of scope here, maybe a followup issue?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review

#4: 1825592-3.simpletest-die-on-fail-and-repeat.patch queued for re-testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
1825592-9.simpletest-die-on-fail-and-repeat.patch5.38 KB
3-9-interdiff.txt1.02 KB

I'm inclined to agree with xjm that adding the option to the admin interface is out of scope. Especially since once set in the admin interface it'll be set for all test runs which is unlikely to be the desired outcome. So we'll maybe we'll need to add it to the re-run tests page... or something like that.

Patch attached improves the run-tests.sh message by providing the db prefix and files directory to make it easier to update your settings to debug the fail.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Hm. I guess we can defer it. It's very odd though to me for run-tests.sh and simpletest.module to be out of sync regarding the features they provide. :\ We're adding a new CMI value here, it seems natural to expose a UI for it.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Reviewed & tested by the community

There's already very different featuresets in run-tests.sh versus the module UI, so I've filed #1893930: Expose a setting to die on failure in the SimpleTest UI

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Sorry, this fell off my radar. This has already been very field-tested in debugging several major issues so it's good to go.

Committed and pushed to 8.x, moving back to 7.x for backport.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs review
FileSize
drupal8.test-random-fail.13.patch2.38 KB

Thanks, this is a useful addition.

Marked #1893930: Expose a setting to die on failure in the SimpleTest UI as won't fix, with a short explanation for why.

Attached patch cleans up the committed changes.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal8.test-random-fail.14.patch3.33 KB

Sorry, public vs. protected.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
14-15-interdiff.txt1.59 KB
1825592.drupal8.test-random-fail.15.patch4.72 KB

Really like the changes in #14 - whilst trying to debug random upgrade failures I wanted to use the --verbose flag as well but discovered that --die-on-fail and --verbose are mutually exclusive... the patch attached fixes that.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to 8.x. Will push in a few mins.

Back to 7.x.

pietmarcus’s picture

pietmarcus CreditAttribution: pietmarcus as a volunteer commented
Assigned: Unassigned » pietmarcus
Issue summary: View changes
Status: Patch (to be ported) » Needs review
FileSize
1825592-18.patch4.57 KB

I had to dust off the patch a bit, but I have backported it to drupal 7. Please review.

Status: Needs review » Needs work

The last submitted patch, 18: 1825592-18.patch, failed testing.

pietmarcus’s picture

pietmarcus CreditAttribution: pietmarcus as a volunteer commented
Status: Needs work » Needs review

Tests ran successfully this time. Please review.

  • webchick committed 8e37c34 on 8.3.x 
    Issue #1825592 by alexpott: Add --repeat and --die-on-fail to run-tests....
  • webchick committed 0aa731a on 8.3.x
    Issue #1825592 follow-up by sun, alexpott: Clean-ups to --repeat and --...

  • webchick committed 8e37c34 on 8.3.x 
    Issue #1825592 by alexpott: Add --repeat and --die-on-fail to run-tests....
  • webchick committed 0aa731a on 8.3.x
    Issue #1825592 follow-up by sun, alexpott: Clean-ups to --repeat and --...

  • webchick committed 8e37c34 on 8.4.x 
    Issue #1825592 by alexpott: Add --repeat and --die-on-fail to run-tests....
  • webchick committed 0aa731a on 8.4.x
    Issue #1825592 follow-up by sun, alexpott: Clean-ups to --repeat and --...

  • webchick committed 8e37c34 on 8.4.x 
    Issue #1825592 by alexpott: Add --repeat and --die-on-fail to run-tests....
  • webchick committed 0aa731a on 8.4.x
    Issue #1825592 follow-up by sun, alexpott: Clean-ups to --repeat and --...
MustangGB’s picture

MustangGB CreditAttribution: MustangGB commented

Would be super handy to have these in D7, anyone able to review?