Here http://qa.drupal.org/pifr/test/483348 all the tests are green, but there is an exception in results (click review log and scroll down):

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'line' at row 3: INSERT INTO {simpletest} (test_id, test_class, status, message, message_group, function, line, file) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7), (:db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14, :db_insert_placeholder_15), (:db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19, :db_insert_placeholder_20, :db_insert_placeholder_21, :db_insert_placeholder_22, :db_insert_placeholder_23); Array
(
    [:db_insert_placeholder_0] => 821
    [:db_insert_placeholder_1] => Drupal\Tests\Component\PhpStorage\FileStorageTest
    [:db_insert_placeholder_2] => pass
    [:db_insert_placeholder_3] =>
    [:db_insert_placeholder_4] => Other
    [:db_insert_placeholder_5] => Drupal\Tests\Component\PhpStorage\FileStorageTest->testCRUD()
    [:db_insert_placeholder_6] => 42
    [:db_insert_placeholder_7] => /var/lib/drupaltestbot/sites/default/files/checkout/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php
    [:db_insert_placeholder_8] => 821
    [:db_insert_placeholder_9] => Drupal\Tests\Component\PhpStorage\FileStorageTest
    [:db_insert_placeholder_10] => pass
    [:db_insert_placeholder_11] =>
    [:db_insert_placeholder_12] => Other
    [:db_insert_placeholder_13] => Drupal\Tests\Component\PhpStorage\FileStorageTest->testReadOnly()
    [:db_insert_placeholder_14] => 51
    [:db_insert_placeholder_15] => /var/lib/drupaltestbot/sites/default/files/checkout/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php
    [:db_insert_placeholder_16] => 821
    [:db_insert_placeholder_17] =>
    [:db_insert_placeholder_18] => pass
    [:db_insert_placeholder_19] =>
    [:db_insert_placeholder_20] => Other
    [:db_insert_placeholder_21] => ->Drupal\Tests\Core\Route\RouterRoleTest::testRoleAccess()
    [:db_insert_placeholder_22] =>
    [:db_insert_placeholder_23] =>
)
 in Drupal\Core\Database\Connection->query() (line 554 of /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php).].
[16:22:48] Command [/usr/bin/php ./core/scripts/run-tests.sh --php /usr/bin/php --url 'http://drupaltestbot659-mysql/checkout' --all --list 2>&1] succeeded.
[16:22:50] Review complete. test_id=483348 result code=10 details=Array
(
    [@pass] => 53909
    [@fail] => 0
    [@exception] => 0
    [@debug] => 142
)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi’s picture

Title: Problem with PHPUnit test results when using @dataProvider » simpletest_phpunit_xml_to_rows() is completely broken with PHPUnit UnitTestCase
Priority: Normal » Major
FileSize
1.66 KB

So I tried to debug this, but it does not make much sense to me.

Here is a patch to use on top of the latest patch from #1956896: Add role based access checker. It prints some debugging information of the returned PHPUnit XML file, which does not fit at all into what is expected in simpletest_phpunit_xml_to_rows().

Execute the test case with.

php core/scripts/run-tests.sh --class "Drupal\Tests\Core\Route\RouterRoleTest"

And observe this SimpleXML testcase element:

SimpleXMLElement Object
(
    [@attributes] => Array
        (
            [name] => Drupal\Tests\Core\Route\RouterRoleTest::testRoleAccess
            [tests] => 3
            [assertions] => 5
            [failures] => 0
            [errors] => 0
            [time] => 0.009941
        )

    [testcase] => Array
        (
            [0] => SimpleXMLElement Object
                (
                    [@attributes] => Array
                        (
                            [name] => testRoleAccess with data set #0
                            [assertions] => 2
                            [time] => 0.007022
                        )

                )

            [1] => SimpleXMLElement Object
                (
                    [@attributes] => Array
                        (
                            [name] => testRoleAccess with data set #1
                            [assertions] => 2
                            [time] => 0.001556
                        )

                )

            [2] => SimpleXMLElement Object
                (
                    [@attributes] => Array
                        (
                            [name] => testRoleAccess with data set #2
                            [assertions] => 1
                            [time] => 0.001363
                        )

                )

        )

)

Does not match $testcase->failure and all the other properties used in simpletest_phpunit_xml_to_rows().

msonnabaum’s picture

Title: simpletest_phpunit_xml_to_rows() is completely broken with PHPUnit UnitTestCase » Problem with PHPUnit test results when using @dataProvider
Status: Active » Needs review
FileSize
2.91 KB

Original title was correct.

Looks like @dataProvider changes the xml format a bit. Attached patch accounts for this and also decomposes that function to make it a bit easier to work with.

Status: Needs review » Needs work

The last submitted patch, 1963022-phpunit-account-for-data-provider.patch, failed testing.

msonnabaum’s picture

Status: Needs work » Needs review
moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Looks like a good bug fix and code refactor.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

A few minor things I noticed when I was going through...

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+      // The file element won't be on testcase objects created with ¶

(nitpick) Trailing whitespace.

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+      $file = (string)$suite->attributes()->file;
...
+    'test_class' => (string)$attributes->class,

(nitpick) Space after (string)

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+function simpletest_phpunit_find_testcases($testsuite) {
...
+function simpletest_phpunit_testcase_to_row($test_id, $testcase, $file) {

Both of these need Doxygen.

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+    if ($testcase->getName() === 'testcase') {

Hm. What else would it be?

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+    elseif (isset($testcase->testcase) && $testcase->tests > 0) {
+      foreach ($testcase->testcase as $childtestcase) {

Looks like $testcase->testcase should be $testcase->testcases, since it's apparently an array of testcases?

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.12 KB
3.63 KB

Fixed some of the docs and the actual functionality.

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,60 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+    elseif (isset($testcase->testcase) && $testcase->tests > 0) {
+      foreach ($testcase->testcase as $childtestcase) {

Looks like $testcase->testcase should be $testcase->testcases, since it's apparently an array of testcases?

Not sure about this point, but as far as i understand this is coming from phpunit.

msonnabaum’s picture

Angie - the ambiguity there is part of why this was a bug. I have to call $testcase->getName() because that's the first level that we can expect testcase elements, but it's possible that it's another testsuite element, in which case it falls through to the else where we treat its children as testcases.

I extracted all other logic from that function so that this bit of complexity is at least isolated, but I'm not sure what else to do about it. I think that plus @dawehner's comment make it clear enough.

olli’s picture

+++ b/core/modules/simpletest/simpletest.module
@@ -756,31 +756,83 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+    elseif (isset($testcase->testcase) && $testcase->tests > 0) {

Should that be $testcase['tests'], or maybe something like:

+    if ($testcase->getName() === 'testsuite') {
+      foreach ($testcase as $childtestcase) {
+        $testcases[] = $childtestcase;
+      }
+    }
+    else {
+      $testcases[] = $testcase;
+    }

?

Is it possible to write a test for this code or submit a dummy test using data provider against the bot to show how error/failure/pass is reported?

olli’s picture

#7: drupal-1963022-7.patch queued for re-testing.

I didn't know we already have a test with data provider.

dawehner’s picture

@olli
Your logic breaks the current phpunit test for simpletest and seems wrong.

In general i'm a bit confused why we need this logic. When I do run the testRoleAccess test I get the following xml:

    <testsuite name="Drupal\Tests\Core\Route\RouterRoleTest::testRoleAccess" tests="3" assertions="5" failures="0" errors="0" time="0.006110">
      <testcase name="testRoleAccess with data set #0" assertions="2" time="0.003302"/>
      <testcase name="testRoleAccess with data set #1" assertions="2" time="0.001520"/>
      <testcase name="testRoleAccess with data set #2" assertions="1" time="0.001288"/>
    </testsuite>

Which not really differs from the other one.

msonnabaum’s picture

Compare that XML to that generated by only running one of the other tests. It should be one less level of nesting.

dawehner’s picture

FileSize
4.58 KB
8.21 KB

Oh you are absolute right. Sometimes it's just good to start with a fresh brain a bit later.

moshe weitzman’s picture

The XML in the last interdiff has some absolute paths in it that are specific to linux. Is that supposed to there?

<failure type="PHPUnit_Framework_ExpectationFailedException">Drupal\Tests\Core\Route\RouterRoleTest::testRoleAccess with data set #0 ('role_test_1', array(Drupal\user\Plugin\Core\Entity\User, Drupal\user\Plugin\Core\Entity\User))
+          Access granted for user with the roles role_test_1 on path: role_test_1
+          Failed asserting that false is true.
+
+          /var/www/d8/core/tests/Drupal/Tests/Core/Route/RouterRoleTest.php:163
+          /usr/share/php/PHPUnit/TextUI/Command.php:176
+          /usr/share/php/PHPUnit/TextUI/Command.php:129
+        </failure>
dawehner’s picture

FileSize
2.32 KB
7.64 KB

Well yeah it's just an error message coming from PHPUNIT. The current file also contains some paths to chx drupal directory.

I just removed the references in the error message.

Status: Needs review » Needs work

The last submitted patch, drupal-1963022-15.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.64 KB
ParisLiakos’s picture

Issue tags: +PHPUnit
+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,83 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+      $file = (string)$suite->attributes()->file;

space missing

+++ b/core/modules/simpletest/simpletest.moduleundefined
@@ -756,31 +756,83 @@ function simpletest_phpunit_xml_to_rows($test_id, $phpunit_xml_file) {
+    'test_class' => (string)$attributes->class,

same

also..tagging

dawehner’s picture

FileSize
1.3 KB
7.65 KB

Cool, here is an update.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

+1 for fixing inline comment:)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

dawehner’s picture

Status: Fixed » Needs review
FileSize
5.41 KB

Oh man, everytime I rely just on tests I fail. That's a trait off between sleep and d.o work.

The changes in the test xml just moves the testsuite one level deeper.

webchick’s picture

Uh. How on earth did that pass before?!

dawehner’s picture

Because the tests xml file got copied wrong :((((( I don't trust myself these days anymore.

dawehner’s picture

One other thing I noticed: Even for all other phpunit tests you can't execute the same test twice. Just tried it out, and this behavior wasn't caused by #19

ParisLiakos’s picture

xml--
well, it is green now, but does it work?
bot reports
PHPUnit errors 1 pass, 0 fails, and 0 exceptions

dawehner’s picture

Well, as you said in IRC yesterday PHPUnit just reports failures in general. In general you get
one result in the XML of PHPUnit per test method, so that's all we can provide at the moment.

ParisLiakos’s picture

well, yes it does not report assertion messages in success, just on failures, but that doesnt mean that it doesnt report the number of successful assertions..locally i get this in the result xml:

<testsuite name="Drupal Unit Test Suite" tests="37" assertions="192" failures="0" errors="0" time="1.091805">

Berdir’s picture

Simpletest doesn't log amounts of assertions, it logs actual messages and then reports the amount of those. We would have to write a dummy assertion for every successfull assertion.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

well lets fix the xml bug here and improve the reports on another issue

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Confirmed that #22 fixes the issue using xdebug.

Committed f9a4d69 and pushed to 8.x. Thanks!

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