Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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
)
Comment | File | Size | Author |
---|---|---|---|
#22 | drupal-1963022-22.patch | 5.41 KB | dawehner |
#19 | drupal-1963022-19.patch | 7.65 KB | dawehner |
#19 | interdiff.txt | 1.3 KB | dawehner |
#17 | drupal-1963022-17.patch | 7.64 KB | dawehner |
#15 | drupal-1963022-15.patch | 7.64 KB | dawehner |
Comments
Comment #1
klausiSo 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.
And observe this SimpleXML testcase element:
Does not match $testcase->failure and all the other properties used in simpletest_phpunit_xml_to_rows().
Comment #2
msonnabaum CreditAttribution: msonnabaum commentedOriginal 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.
Comment #4
msonnabaum CreditAttribution: msonnabaum commented#2: 1963022-phpunit-account-for-data-provider.patch queued for re-testing.
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedLooks like a good bug fix and code refactor.
Comment #6
webchickA few minor things I noticed when I was going through...
(nitpick) Trailing whitespace.
(nitpick) Space after (string)
Both of these need Doxygen.
Hm. What else would it be?
Looks like $testcase->testcase should be $testcase->testcases, since it's apparently an array of testcases?
Comment #7
dawehnerFixed some of the docs and the actual functionality.
Not sure about this point, but as far as i understand this is coming from phpunit.
Comment #8
msonnabaum CreditAttribution: msonnabaum commentedAngie - 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.
Comment #9
olli CreditAttribution: olli commentedShould that be $testcase['tests'], or maybe something like:
?
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?
Comment #10
olli CreditAttribution: olli commented#7: drupal-1963022-7.patch queued for re-testing.
I didn't know we already have a test with data provider.
Comment #11
dawehner@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:
Which not really differs from the other one.
Comment #12
msonnabaum CreditAttribution: msonnabaum commentedCompare that XML to that generated by only running one of the other tests. It should be one less level of nesting.
Comment #13
dawehnerOh you are absolute right. Sometimes it's just good to start with a fresh brain a bit later.
Comment #14
moshe weitzman CreditAttribution: moshe weitzman commentedThe XML in the last interdiff has some absolute paths in it that are specific to linux. Is that supposed to there?
Comment #15
dawehnerWell 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.
Comment #17
dawehnerThis conflicted with #1969406: PHPUnit Test asserting logging broken
Comment #18
ParisLiakos CreditAttribution: ParisLiakos commentedspace missing
same
also..tagging
Comment #19
dawehnerCool, here is an update.
Comment #20
ParisLiakos CreditAttribution: ParisLiakos commented+1 for fixing inline comment:)
Comment #21
webchickCommitted and pushed to 8.x. Thanks!
Comment #22
dawehnerOh 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.
Comment #23
webchickUh. How on earth did that pass before?!
Comment #24
dawehnerBecause the tests xml file got copied wrong :((((( I don't trust myself these days anymore.
Comment #25
dawehnerOne 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
Comment #26
ParisLiakos CreditAttribution: ParisLiakos commentedxml--
well, it is green now, but does it work?
bot reports
PHPUnit errors 1 pass, 0 fails, and 0 exceptions
Comment #27
dawehnerWell, 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.
Comment #28
ParisLiakos CreditAttribution: ParisLiakos commentedwell, 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">
Comment #29
BerdirSimpletest 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.
Comment #30
ParisLiakos CreditAttribution: ParisLiakos commentedwell lets fix the xml bug here and improve the reports on another issue
Comment #31
alexpottConfirmed that #22 fixes the issue using xdebug.
Committed f9a4d69 and pushed to 8.x. Thanks!