Moodle

Some phpunit tests are not being run as missing in phpunit.xml

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.3.4, 2.4.1, 2.5
  • Fix Version/s: 2.3.5, 2.4.2
  • Component/s: Unit tests
  • Labels:

Description

As discovered with MDLSITE-1972, there are some tests which are never being run in the main phpunit run.

Issue Links

caused a regression

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37694 Move reportlib_test.php to mod/quiz/tests/

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
is duplicated by

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37437 phpunit.xml: add all external test files

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
testing discovered

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37086 message/tests/externallib_test.php is broken

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37694 Move reportlib_test.php to mod/quiz/tests/

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
will help resolve

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDLSITE-1972 Detect tests without entry in phpunit.xml.dist

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dan Poltawski added a comment - - edited

Clearly the need for adding these phpunit.xml.dist entries was not made clear to people hence so many missing. Some potential ideas were discussed in MDLSITE-1972.

Show
Dan Poltawski added a comment - - edited Clearly the need for adding these phpunit.xml.dist entries was not made clear to people hence so many missing. Some potential ideas were discussed in MDLSITE-1972 .
Hide
Permalink
Dan Poltawski added a comment -

Tim/Petr/Jerome - added you all here FYI because I think that you introduced tests without the phpunit.xml.dist entries.

2.3 missing:
http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(23_STABLE)/223/console

ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml !
ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml !
ERROR: rating/tests is not matched/covered by any definition in phpunit.xml !
ERROR: repository/tests is not matched/covered by any definition in phpunit.xml !
ERROR: user/tests is not matched/covered by any definition in phpunit.xml !
ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml !

2.4 Missing
http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(24_STABLE)/194/console

ERROR: enrol/tests is not matched/covered by any definition in phpunit.xml !
ERROR: group/tests is not matched/covered by any definition in phpunit.xml !
ERROR: lib/external/tests is not matched/covered by any definition in phpunit.xml !
ERROR: message/tests is not matched/covered by any definition in phpunit.xml !
ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml !
ERROR: notes/tests is not matched/covered by any definition in phpunit.xml !
ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml !
ERROR: rating/tests is not matched/covered by any definition in phpunit.xml !
ERROR: repository/tests is not matched/covered by any definition in phpunit.xml !
ERROR: user/tests is not matched/covered by any definition in phpunit.xml !
ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml !

master missing:

http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(master)/372/console

ERROR: enrol/tests is not matched/covered by any definition in phpunit.xml !
ERROR: group/tests is not matched/covered by any definition in phpunit.xml !
ERROR: lib/external/tests is not matched/covered by any definition in phpunit.xml !
ERROR: message/tests is not matched/covered by any definition in phpunit.xml !
ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml !
ERROR: notes/tests is not matched/covered by any definition in phpunit.xml !
ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml !
ERROR: rating/tests is not matched/covered by any definition in phpunit.xml !
ERROR: repository/tests is not matched/covered by any definition in phpunit.xml !
ERROR: user/tests is not matched/covered by any definition in phpunit.xml !
ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml !

Show
Dan Poltawski added a comment - Tim/Petr/Jerome - added you all here FYI because I think that you introduced tests without the phpunit.xml.dist entries. 2.3 missing: http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(23_STABLE)/223/console ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml ! ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml ! ERROR: rating/tests is not matched/covered by any definition in phpunit.xml ! ERROR: repository/tests is not matched/covered by any definition in phpunit.xml ! ERROR: user/tests is not matched/covered by any definition in phpunit.xml ! ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml ! 2.4 Missing http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(24_STABLE)/194/console ERROR: enrol/tests is not matched/covered by any definition in phpunit.xml ! ERROR: group/tests is not matched/covered by any definition in phpunit.xml ! ERROR: lib/external/tests is not matched/covered by any definition in phpunit.xml ! ERROR: message/tests is not matched/covered by any definition in phpunit.xml ! ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml ! ERROR: notes/tests is not matched/covered by any definition in phpunit.xml ! ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml ! ERROR: rating/tests is not matched/covered by any definition in phpunit.xml ! ERROR: repository/tests is not matched/covered by any definition in phpunit.xml ! ERROR: user/tests is not matched/covered by any definition in phpunit.xml ! ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml ! master missing: http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(master)/372/console ERROR: enrol/tests is not matched/covered by any definition in phpunit.xml ! ERROR: group/tests is not matched/covered by any definition in phpunit.xml ! ERROR: lib/external/tests is not matched/covered by any definition in phpunit.xml ! ERROR: message/tests is not matched/covered by any definition in phpunit.xml ! ERROR: mod/quiz/report/tests is not matched/covered by any definition in phpunit.xml ! ERROR: notes/tests is not matched/covered by any definition in phpunit.xml ! ERROR: question/engine/upgrade/tests is not matched/covered by any definition in phpunit.xml ! ERROR: rating/tests is not matched/covered by any definition in phpunit.xml ! ERROR: repository/tests is not matched/covered by any definition in phpunit.xml ! ERROR: user/tests is not matched/covered by any definition in phpunit.xml ! ERROR: webservice/tests is not matched/covered by any definition in phpunit.xml !
Hide
Permalink
Dan Poltawski added a comment -

Apologies, I think some of these are false positives..

Show
Dan Poltawski added a comment - Apologies, I think some of these are false positives..
Hide
Permalink
Dan Poltawski added a comment - - edited

I've created a patch for master, although i'm not quite sure i've got the testsuite names right:
https://github.com/danpoltawski/moodle/compare/dec70f2...MDL-37684

Before:

Tests: 1587, Assertions: 29207, Skipped: 1.

After:

There was 1 error:

1) core_message_external_testcase::test_send_instant_messages
dml_transaction_exception: Database transaction error (This code can not be excecuted in transaction)

/Users/danp/git/integration/lib/dml/moodle_database.php:2136
/Users/danp/git/integration/lib/messagelib.php:61
/Users/danp/git/integration/message/lib.php:2057
/Users/danp/git/integration/message/externallib.php:147
/Users/danp/git/integration/message/tests/externallib_test.php:87
/Users/danp/git/integration/lib/phpunit/classes/advanced_testcase.php:76

To re-run:
 vendor/bin/phpunit core_message_external_testcase message/tests/externallib_test.php

--

There was 1 failure:

1) mod_quiz_reportlib_testcase::test_quiz_report_index_by_keys
Failed asserting that 4 matches expected 3.

/Users/danp/git/integration/mod/quiz/report/tests/reportlib_test.php:51
/Users/danp/git/integration/lib/phpunit/classes/basic_testcase.php:64

To re-run:
 vendor/bin/phpunit mod_quiz_reportlib_testcase mod/quiz/report/tests/reportlib_test.php

FAILURES!
Tests: 1619, Assertions: 29807, Failures: 1, Errors: 1, Skipped: 1.
Show
Dan Poltawski added a comment - - edited I've created a patch for master, although i'm not quite sure i've got the testsuite names right: https://github.com/danpoltawski/moodle/compare/dec70f2...MDL-37684 Before: Tests: 1587, Assertions: 29207, Skipped: 1. After: There was 1 error: 1) core_message_external_testcase::test_send_instant_messages dml_transaction_exception: Database transaction error (This code can not be excecuted in transaction) /Users/danp/git/integration/lib/dml/moodle_database.php:2136 /Users/danp/git/integration/lib/messagelib.php:61 /Users/danp/git/integration/message/lib.php:2057 /Users/danp/git/integration/message/externallib.php:147 /Users/danp/git/integration/message/tests/externallib_test.php:87 /Users/danp/git/integration/lib/phpunit/classes/advanced_testcase.php:76 To re-run: vendor/bin/phpunit core_message_external_testcase message/tests/externallib_test.php -- There was 1 failure: 1) mod_quiz_reportlib_testcase::test_quiz_report_index_by_keys Failed asserting that 4 matches expected 3. /Users/danp/git/integration/mod/quiz/report/tests/reportlib_test.php:51 /Users/danp/git/integration/lib/phpunit/classes/basic_testcase.php:64 To re-run: vendor/bin/phpunit mod_quiz_reportlib_testcase mod/quiz/report/tests/reportlib_test.php FAILURES! Tests: 1619, Assertions: 29807, Failures: 1, Errors: 1, Skipped: 1.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

+1 for immediate integration of this. It's better to have them running ASAP (no matter the 1 error & 1 warning) than ignoring them.

Show
Eloy Lafuente (stronk7) added a comment - +1 for immediate integration of this. It's better to have them running ASAP (no matter the 1 error & 1 warning) than ignoring them.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Integrated (23, 24 & master), thanks!

Show
Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Passing, on behalf of the CI server results, hopefully now executing the phpunit tests.

Show
Eloy Lafuente (stronk7) added a comment - Passing, on behalf of the CI server results, hopefully now executing the phpunit tests.
Hide
Permalink
Tim Hunt added a comment -

This bit is wrong:

<testsuite name="quiz_report">	
    <directory suffix="_test.php">mod/quiz/report/tests</directory>
</testsuite>

The correct fix there was to move mod/quiz/report/tests/reportlib_test.php to mod/quiz/tests/reportlib_test.php, since that is how Petr decided tests for plugins should work (all in one folder).

Do you want to integrate one more commit to fix that, or will you file a new MDL?

Show
Tim Hunt added a comment - This bit is wrong: <testsuite name= "quiz_report" > <directory suffix= "_test.php" > mod/quiz/report/tests </directory> </testsuite> The correct fix there was to move mod/quiz/report/tests/reportlib_test.php to mod/quiz/tests/reportlib_test.php, since that is how Petr decided tests for plugins should work (all in one folder). Do you want to integrate one more commit to fix that, or will you file a new MDL?
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

I'd create new MDLs for everything (the failing ones and the issue you comment). Let the CI servers finish running them and the Tracker will be spread, oki?

Show
Eloy Lafuente (stronk7) added a comment - I'd create new MDLs for everything (the failing ones and the issue you comment). Let the CI servers finish running them and the Tracker will be spread, oki?
Hide
Permalink
Eloy Lafuente (stronk7) added a comment - - edited

Oki, so the only things following this are:

  • The move of mod/quiz/report/tests/reportlib_test.php reported by Tim (MDL-37694 created)
  • Problems under Postgres with messaging tests. Already being handled @ MDL-37086

Ciao

Edited: Created MDL-37694

Show
Eloy Lafuente (stronk7) added a comment - - edited Oki, so the only things following this are: The move of mod/quiz/report/tests/reportlib_test.php reported by Tim ( MDL-37694 created) Problems under Postgres with messaging tests. Already being handled @ MDL-37086 Ciao Edited: Created MDL-37694
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

Closing as fixed, ciao

Show
Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

People

Vote (0)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: