Problem/Motivation

While working on: #2605664: [Needs change record updates] Align TestDiscovery and bootstrap.php's non-permissive loading of PSR-4 namespaces for traits

MissingDependentModuleUnitTest kept failing while I was running the simpletest group with the phpunit binary.

But it never failed when I ran run-tests.sh.

Turns out it has the wrong namespace declaration, so run-tests.sh ignored it.

Proposed resolution

Change the namespace.

Deal with the failing test.

Remaining tasks

Have run-tests.sh/TestDiscovery throw an exception when tests don't conform, in order to prevent false positives.

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 convert-2722731-7.patch720 bytesneclimdul
#4 2721355-4.patch534 bytesalexpott
#4 2721355-4.test-only.patch724 bytesalexpott
#2 2721355_2.patch453 bytesMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2721355_2.patch453 bytes

Simple change of namespace.

Unfortunately this leads to a failing test, which I'm not sure how to fix, so the testbot should fail.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

Right, so the testbot *still* didn't find it, even though run-tests.sh finds it for group simpletest.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
2721355-4.test-only.patch724 bytes
2721355-4.patch534 bytes

So the whole point is that the test is supposed to not be discovered because of the @dependencies annotation.

The last submitted patch, 4: 2721355-4.test-only.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs review » Needs work

Well... that's not going to work in phpunit.

SIMPLETEST_DB='...' ./vendor/bin/phpunit -c ./core/phpunit.xml.dist --testsuite=kernel --stop-on-fail
PHPUnit 4.8.11 by Sebastian Bergmann and contributors.

.............................................................   61 / 1325 (  4%)
.............................................................  122 / 1325 (  9%)
.............................................................  183 / 1325 ( 13%)
.............................................................  244 / 1325 ( 18%)
.............................................................  305 / 1325 ( 23%)
.............................................................  366 / 1325 ( 27%)
.............................................................  427 / 1325 ( 32%)
.............................................................  488 / 1325 ( 36%)
..............................F
Time: 14.5 minutes, Memory: 38.00Mb

There was 1 failure:

1) Drupal\simpletest\Tests\MissingDependentModuleUnitTest::testFail
Running test with missing required module.

FAILURES!
Tests: 519, Assertions: 7809, Failures: 1.

Maybe the real fix is this should be moved back to simpletest since its testing its discovery.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs work » Reviewed & tested by the community

You know what, I'll follow up with that separately. This is fine.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work

@neclimdul actually I think your real fix is the right fix :)

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented

this one? #2722731: Mark MissingDependentModuleUnitTest as incomplete

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs work » Needs review
FileSize
convert-2722731-7.patch720 bytes

This had the original patch actually so actually we can solve it here. I meant to solve a different issue in the other issue as a follow but we can solve it all here.

Filed a follow up for actual phpunit module dependency support since KTB doesn't link to one in its TODO and I couldn't find one. #2728579: Explicitly skip @requires module in PHPUnit Kernel and Browser tests

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

I don't see where MissingDependentModuleUnitTest ran on the testbot, which is apparently the goal, so therefore yay!

Unfortunately that also means we can't see whether it was actually rejected or just didn't run due to some other error. The solution from the other issue is actually a better one in terms of testing, where we can see a skipped test.

So it's not a great test, but at least we solved the namespace problem.

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented

Right, simpletest doesn't have the same "skipped" idea. There is an issue to implement that or an "expected failure" or both but I doubt there is much momentum behind it at this point so this is fine.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: convert-2722731-7.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs work » Reviewed & tested by the community

The fact that Drupal\migrate_drupal_ui\Tests\d6\MigrateUpgrade6Test can ruin the status of so many issues makes me never want to migrate my old D6 site. :-)

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented

90% sure its a bug in sqlite libraries or filesystem issues.

  • catch committed ad7cb93 on 8.2.x 
    Issue #2721355 by alexpott, neclimdul, Mile23:...

  • catch committed dcf2f67 on 8.1.x 
    Issue #2721355 by alexpott, neclimdul, Mile23:...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

Status: Fixed » Closed (fixed)

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