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).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JacobSingh’s picture

Yeah 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.

JacobSingh’s picture

Status: Active » Needs review
TR’s picture

Specifically, 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:

Detailed test results:
----------------------



---- BookTestCase ----


Pass       Other      book.test                      27                        
    Enabled modules: book
Pass       Role       book.test                      30                        
    Created role of name: iKTJNjP3, id: 4
Pass       Role       book.test                      30                        
    Created permissions: create new books, create book content, edit own book
    content, add content to books
[etc.]

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:

Detailed test results
---------------------


---- BookTestCase ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Pass      Other      book.test           27 BookTestCase->setUp()              
    Enabled modules: book
Pass      Role       book.test           30 BookTestCase->setUp()              
    Created role of name: zp86MI0x, id: 4
Pass      Role       book.test           30 BookTestCase->setUp()              
    Created permissions: create new books, create book content, edit own book
    content, add content to books
[etc.]





With wrong patch from #1, for comparison:

Detailed test results:
----------------------



---- BookTestCase ----


Pass       Other      book.test                      27    run-tests.sh        
    Enabled modules: book
Pass       Role       book.test                      30    run-tests.sh        
    Created role of name: kzO5s5dy, id: 4
Pass       Role       book.test                      30    run-tests.sh        
    Created permissions: create new books, create book content, edit own book
    content, add content to books
[etc.]
TR’s picture

Version: 7.x-dev » 8.x-dev
Assigned: Unassigned » TR
Issue tags: +Needs backport to D7

Note, the patch applies cleanly to both D8 and D7.

Status: Needs review » Needs work
Issue tags: -Needs backport to D7

The last submitted patch, 666854-run-tests-verbose.patch, failed testing.

TR’s picture

Status: Needs work » Needs review
Issue tags: +Needs backport to D7

#3: 666854-run-tests-verbose.patch queued for re-testing.

Let's try that again - the failing tests runs green locally for me.

pillarsdotnet’s picture

Issue tags: +Needs backport to D7

+1

TR’s picture

#3: 666854-run-tests-verbose.patch queued for re-testing.

xjm’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Thanks 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.

TR’s picture

Status: Needs work » Needs review
FileSize
1.75 KB

The 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

xjm’s picture

Issue tags: -Novice

Thanks 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!

xjm’s picture

xjm’s picture

Status: Needs review » Reviewed & tested by the community

Alright, I think we are good here then.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x and back-ported to 7.x.

sun’s picture

Status: Fixed » Needs work
+++ b/core/scripts/run-tests.sh
@@ -598,6 +597,10 @@ function simpletest_script_reporter_display_results() {
           $test_class = $result->test_class;
+
+  // Print table header.
+  echo "Status    Group      Filename          Line Function                            \n";
+  echo "--------------------------------------------------------------------------------\n";
         }

This 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...?

TR’s picture

What'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.

TR’s picture

Oh, 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.

TR’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 666854-indentation.patch, failed testing.

TR’s picture

TR’s picture

Status: Needs work » Needs review

Let's try that again now that head is fixed.

TR’s picture

#17: 666854-indentation.patch queued for re-testing.

xjm’s picture

Status: Needs review » Reviewed & tested by the community

The indentation fix looks fine.

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review

Committed/pushed to 8.x, moving back to 7.x.

TR’s picture

FileSize
840 bytes

I'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.

xjm’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D7

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