Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Comment | File | Size | Author |
---|---|---|---|
#29 | test-keep-results-1719530-28.patch | 2.69 KB | alberto56 |
#26 | test-keep-results-1719530-26.patch | 2.67 KB | dcam |
#24 | test-keep-results-1719530-24.patch | 2.68 KB | dcam |
#15 | test.keep-results.15.patch | 1.02 KB | sun |
#9 | drupal8.run-tests-keep-results.9.patch | 952 bytes | sun |
Comments
Comment #1
tim.plunkettSee attached
Comment #2
boombatower CreditAttribution: boombatower commentedYea, as I suggested committing this first will make it easier to be sure the pifr change works before we commit the actual core change so we don't end up with the disruption we had yesterday with the commit.
Comment #3
sunI can't RTBC my own patch.
Comment #4
tim.plunkettI can do that. (I should have mentioned above, I didn't write a single line of this, sun did).
Comment #5
Dries CreditAttribution: Dries commentedIt would be nicer if the command line settings where consistent with the settings in the Simpletest configuration UI.
I prefer
--keep-results
over--clear-results
so maybe we should consider renaming the internal variable tosimpletest_clear_results
tosimpletest_keep_results
and updating the configuration UI accordingly? It would also make the code a tiny bit cleaner.Comment #6
Dries CreditAttribution: Dries commentedI don't want to hold up this issue on #5 so I decided to go ahead and commit it.
If people feel #5 is valuable, maybe we can create a follow-up patch or a new issue? Marking this issue 'fixed' for the time being.
Comment #7
sunThe default, in both cases, should actually be to delete the results. Keeping the assertion results should be the opt-in, not the other way around.
So I'd be fine with patching the Simpletest module to rename its variable/config-key from clear_results to keep_results (as well as adjusting the UI and negating the default value).
Comment #8
sunWe actually need to backport this to D7...
Comment #9
sunOh dear. I'm sorry.
Comment #10
boombatower CreditAttribution: boombatower commentedwas just going to post that I confirmed it was this from the other issue. testing.
Comment #11
boombatower CreditAttribution: boombatower commentedConfirmed locally.
Comment #12
webchickOops. :)
Committed and pushed to 8.x. Thanks!
Moving back to 7.x.
Comment #13
webchickAhem.
Comment #14
sunComment #15
sunFinally discovered why we're getting lots of verbose output in test results -- which lead to many more total test result assertions, and since PIFR has a maximum limit of assertions it displays, this means that actual errors and exceptions of failing tests are not displayed. (example)
This is a stop-gap fix for D8, which can be removed with #1705748: Convert simpletest settings to configuration system
Comment #16
webchickHeh, oops. Since SimpleTest hasn't been converted to CMI yet, these settings currently have no effect. The temporary patch adds them back in under their old D7 names as a stop-gap.
Committed and pushed to 8.x.
Comment #17
sunThanks! :)
Comment #18
boombatower CreditAttribution: boombatower commentedTo be reverted once #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag is committed.
Comment #19
boombatower CreditAttribution: boombatower commentedFor future reference note that this issue also change the definition of --verbose to include controlling the simpletest_verbose variable which controls the capturing the browser requests in DWTC.
To be fixed in one way or another in
#1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag
Comment #20
webchickCorrect me if I'm wrong, but I don't believe this backport is actually major for 7.x; it merely keeps the code bases closer together. Tentatively de-prioritizing so that I can commit me some features. :)
Comment #21
dcam CreditAttribution: dcam commentedWhich of these patches should be backported? I ask because #15 is a "stop-gap fix for D8."
Comment #22
webchickActually, that is a fine question.
Comment #23
sunThere are three patches in this issue that have been committed, and the cumulative result of all three of them should be backported.
Comment #24
dcam CreditAttribution: dcam commentedBackported #1, #9, and #15 to D7.
Comment #26
dcam CreditAttribution: dcam commentedFixed the filepath in line 410 of run-tests.sh that was causing the error in #24.
Comment #27
boombatower CreditAttribution: boombatower commentedIt would seem prudent to get #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag in and add to backport since the patch as it stands introduces an "api change" by changing the meaning of --verbose instead of just two additional flags (only adds one the way it stands and alters meaning of the other).
Comment #29
alberto56 CreditAttribution: alberto56 commentedHere is a version of #26 which applies to 7.22.