[MDL-37684] Some phpunit tests are not being run as missing in phpunit.xml - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: 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:
- triaged

Testing Instructions:

Hide

Tested by the CI server checks of not tested tests.

Show
Tested by the CI server checks of not tested tests.
Affected Branches:
MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
Fixed Branches:
MOODLE_23_STABLE, MOODLE_24_STABLE
Pull from Repository:
git://github.com/danpoltawski/moodle.git
Pull Master Branch:
~~MDL-37684~~
Pull Master Diff URL:
https://github.com/danpoltawski/moodle/compare/a737019...MDL-37684

Description

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

Gliffy Diagrams

Attachments

Issue Links

caused a regression

MDL-37694 Move reportlib_test.php to mod/quiz/tests/

Closed

Discovered while testing

MDL-37086 message/tests/externallib_test.php is broken

Closed

MDL-37694 Move reportlib_test.php to mod/quiz/tests/

Closed

is duplicated by

MDL-37437 phpunit.xml: add all external test files

Closed

will help resolve

MDLSITE-1972 Detect tests without entry in phpunit.xml.dist

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Dan Poltawski added a comment - 25/Jan/13 3:42 AM - 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 - 25/Jan/13 3:42 AM - 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 - 25/Jan/13 3:58 AM

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

Show

Dan Poltawski added a comment - 25/Jan/13 3:58 AM 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 - 25/Jan/13 4:00 AM

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

Show

Dan Poltawski added a comment - 25/Jan/13 4:00 AM Apologies, I think some of these are false positives..

Hide

Permalink

Dan Poltawski added a comment - 25/Jan/13 4:33 AM - 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 - 25/Jan/13 4:33 AM - 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 - 25/Jan/13 8:35 AM

+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 - 25/Jan/13 8:35 AM +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 - 25/Jan/13 8:43 AM

Integrated (23, 24 & master), thanks!

Show

Eloy Lafuente (stronk7) added a comment - 25/Jan/13 8:43 AM Integrated (23, 24 & master), thanks!

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 25/Jan/13 8:43 AM

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

Show

Eloy Lafuente (stronk7) added a comment - 25/Jan/13 8:43 AM Passing, on behalf of the CI server results, hopefully now executing the phpunit tests.

Hide

Permalink

Tim Hunt added a comment - 25/Jan/13 8:58 AM

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 - 25/Jan/13 8:58 AM 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 - 25/Jan/13 9:01 AM

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 - 25/Jan/13 9:01 AM 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 - 25/Jan/13 10:14 AM - 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 - 25/Jan/13 10:14 AM - 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 - 25/Jan/13 11:33 AM

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 - 25/Jan/13 11:33 AM Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

People

Assignee:

Dan Poltawski

Reporter:

Dan Poltawski

Integrator:

Eloy Lafuente (stronk7)

Tester:

Eloy Lafuente (stronk7)

Participants:

Dan Poltawski, Eloy Lafuente (stronk7), Tim Hunt

Votes:

0 Vote for this issue

Watchers:

4 Start watching this issue

Dates

Created:

25/Jan/13 3:35 AM

Updated:

25/Jan/13 11:44 AM

Resolved:

25/Jan/13 11:33 AM

Fix Release Date:

11/Mar/13