Page MenuHomePhabricator

Collection tests do not run properly
Closed, ResolvedPublic5 Story Points

Description

As https://gerrit.wikimedia.org/r/#/c/382854 shows the phpunit tests are not being run on a per commit basis.
@Tgr any ideas to why?

This is blocking progress in the Collection extension.

Outcome

See T177672#3671066 and T177672#3671894.

Investigations

  • Does not seem to be related to directory structure inside test
  • Tests can be run locally outside Vagrant, but do not run inside Vagrant.
  • Tests do not run on commits but integration config for Collection extension looks normal.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 6 2017, 10:22 PM
Tgr added a comment.Oct 6 2017, 10:57 PM

If something is wrong with that, --testsuite extensions probably runs https://github.com/wikimedia/mediawiki/blob/master/tests/phpunit/suites/ExtensionsTestSuite.php so that's where I'd look.

pmiazga added a subscriber: pmiazga.EditedOct 7 2017, 12:24 AM

@Tgr our assumptions comes from the fact that while we worked on RenderBook class we changed the BookRenderer class a lot, we even changed the BookRenderer::renderBook() signature and tests didn't fail which is puzzling.

Example: Tests should fail as BookRenderer::renderBook() was expecting a single argument, a BookData instance, but tests were still passing three arrays (see https://gerrit.wikimedia.org/r/#/c/382609/3/includes/BookRenderer.php).

Jdlrobson triaged this task as High priority.Oct 9 2017, 5:17 PM
Jdlrobson set the point value for this task to 5.

Why do you think so? There is a phpunit block at the end of https://integration.wikimedia.org/ci/job/mwext-testextension-hhvm-jessie/18577/console.

Tests are failing locally. It may be running but it doesn't seem to be locating the tests for some reason...

Jdlrobson updated the task description. (Show Details)Oct 9 2017, 7:32 PM
Tgr added a comment.Oct 10 2017, 3:42 AM

Looks like the test discovery relies on extension registration which Collection does not use. You'll probably have to implement the UnitTestsList hook manually.

Change 383350 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/Collection@master] Register PHPUnit tests via UnitTestsList hook

https://gerrit.wikimedia.org/r/383350

Looks like the test discovery relies on extension registration which Collection does not use. You'll probably have to implement the UnitTestsList hook manually.

@Tgr was correct. This is done in rECOL89f7386e303f: Register PHPUnit tests via UnitTestsList hook.

For the curious, the extension test path discovery mechanism is defined in tests/phpunit/suites/ExtensionsTestSuite.php. Every extension registered with the extension registry has its tests added to the list and then the UnitTestsList hook is run to supplement that list.

I confirm the patch fixes the problem. I had no idea extension.json was needed for automatic test registration. The patch has been +2ed but it's merge is now blocked on T177801 which fixes the tests.

Change 383350 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Register PHPUnit tests via UnitTestsList hook

https://gerrit.wikimedia.org/r/383350

The tests are now run on a per-commit basis 🎉🎉🎉

This task skipped Needs QA because it's purely technical and was verified as part of merging the associated change and those associated with T177801: Collection phpunit tests are failing for table of contents when run locally.

phuedx removed phuedx as the assignee of this task.Oct 10 2017, 6:04 PM
phuedx added a subscriber: phuedx.
phuedx closed this task as Resolved.Oct 11 2017, 2:45 PM
phuedx claimed this task.
phuedx updated the task description. (Show Details)

Being bold.