Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
While running SimpleTest using run_tests.sh, the dblog fills with these notices:
Notice: Undefined property: stdClass::$caller in simpletest_script_format_result() (line 531 of scripts/run-tests.sh).
Comment | File | Size | Author |
---|---|---|---|
#25 | 666854-indentation.patch | 840 bytes | TR |
#17 | 666854-indentation.patch | 860 bytes | TR |
#17 | 666854-indentation-D7.patch | 840 bytes | TR |
#10 | 666854-run-tests-verbose-reroll.patch | 1.75 KB | TR |
#3 | booktest.png | 29.6 KB | TR |
Comments
Comment #1
JacobSingh CreditAttribution: JacobSingh commentedYeah me, too but over a year later :) That column just doesn't exist.
I don't know if this is the desired fix since I don't know what ->caller was there for to begin with.
Comment #2
JacobSingh CreditAttribution: JacobSingh commentedComment #3
TR CreditAttribution: TR commentedSpecifically, this bug is seen only when using the --verbose flag with run-tests.sh.
The fix in #1 is wrong because it prints the name of this script, run-tests.sh, instead of the name of the test function, which is what was intended.
--verbose output prints a text-based table with a row for every test. This table mimics the HTML table that you see when running tests from admin/config/development/testing (see attachment for appearance of HTML table).
The columns of the table are the status of the test (pass/fail), the test group, the .test file name, the line number of the test, and the "caller" as mentioned above. Additionally, a description of the test is printed below each row (see below for example output).
It's true the column "caller" just doesn't exist - the proper name of the column is "function", which will print the name of the test function just as it does if you run tests from admin/config/development/testing.
The attached patch changes ->caller to ->function, adds a table header for readability, and adjust the column widths to make better use of the 80 columns available.
Here's example output from
$ php run-tests.sh Book
so you can see how the patch changes things:Before patch:
Note that there is no description on the columns, the last column is missing, and each row is accompanied by a PHP notice in the watchdog -
Notice: Undefined property: stdClass::$caller in simpletest_script_format_result() (line 615 of /xxx/scripts/run-tests.sh).
After patch:
With wrong patch from #1, for comparison:
Comment #4
TR CreditAttribution: TR commentedNote, the patch applies cleanly to both D8 and D7.
Comment #6
TR CreditAttribution: TR commented#3: 666854-run-tests-verbose.patch queued for re-testing.
Let's try that again - the failing tests runs green locally for me.
Comment #7
pillarsdotnet CreditAttribution: pillarsdotnet commented+1
Comment #8
TR CreditAttribution: TR commented#3: 666854-run-tests-verbose.patch queued for re-testing.
Comment #9
xjmThanks for your work on this patch. Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
Tagging as novice for the task of rerolling the Drupal 8.x patch.
If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.
Comment #10
TR CreditAttribution: TR commentedThe attached patch has been re-rolled for the new Drupal 8.x.
The patch in #3 has identical content, and applies cleanly in the current Drupal 7.x
Comment #11
xjmThanks for the reroll. I'll retest to make sure this still applies. Based on #3 it sounds like this is probably RTBC so long as it applies. Thanks!
Comment #12
xjm#10: 666854-run-tests-verbose-reroll.patch queued for re-testing.
Comment #13
xjmAlright, I think we are good here then.
Comment #14
Dries CreditAttribution: Dries commentedCommitted to 8.x and back-ported to 7.x.
Comment #15
sunThis indentation is completely off.
I'm also not sure what those CLI UI changes have to do with E_NOTICE warnings in run-tests.sh in the first place...?
Comment #16
TR CreditAttribution: TR commentedWhat's "off" about the indentation? I would be happy to deal with any specific problems you point out.
You're right that the formatting of the output doesn't have anything to do with the E_NOTICE warning, and probably should have been separated out into its own issue. And if that had been pointed out five months ago that's what I would have done. But please note that the formatting change was explicitly disclosed up-front with complete "before" and "after" versions shown in the post as well as an image of how the output looks in the browser - nothing has been slipped in under cover of an E_NOTICE issue.
Comment #17
TR CreditAttribution: TR commentedOh, I see what you're saying, the source code is indented wrong. Correct you are. Here is a D8 and D7 patch to fix the indentation problem.
Comment #18
TR CreditAttribution: TR commentedComment #20
TR CreditAttribution: TR commentedTest failed because Drupal 8.x core tests are broken: #61456: Aggregator titles display quotes and other characters with HTML entity equivalents badly (write tests)
Comment #21
TR CreditAttribution: TR commentedLet's try that again now that head is fixed.
Comment #22
TR CreditAttribution: TR commented#17: 666854-indentation.patch queued for re-testing.
Comment #23
xjmThe indentation fix looks fine.
Comment #24
catchCommitted/pushed to 8.x, moving back to 7.x.
Comment #25
TR CreditAttribution: TR commentedI've re-uploaded the D7 version of the patch from #17 and attached it here without the -D7 extension so the testbot can look at it.
Comment #26
xjmComment #27
webchickCommitted and pushed to 7.x. Thanks!