When a test does not enable any module, then each test setUp() adds a needless and bogus assertion:

[pass] Enabled modules: .

CommentFileSizeAuthor
#29 2207377-psr4-reroll.patch957 bytesxjm
#22 drupal8.test-modules-empty.22.patch1 KBsun
#9 drupal8.test-modules-empty.9.patch1.02 KBsun
drupal8.test-modules-empty.0.patch1.03 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

drupal8.test-modules-empty.0.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

drupal8.test-modules-empty.0.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal8.test-modules-empty.0.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review

drupal8.test-modules-empty.0.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Parent issue: » #2215035: [meta] Simpletest UI cleanups
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community

Yup, looks great. Looked at the surrounding code and this makes a lot of sense.

In case this needs a re-roll feel free to remove the second param to enableModules() while we're at it, as that is unused. That's a pre-existing issue, though, so RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 3b3cb6f and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Closed (fixed) » Needs review
FileSize
drupal8.test-modules-empty.9.patch1.02 KB

Sorry, somehow this patch fixed the issue in my original manual testing, but I've encountered these bogus assertion messages again.

The problem is that $modules is actually a nested array, and it always contains a key for each class name. Only the nested values can be empty. For example:

array(
  'Drupal\\simpletest\\DrupalUnitTestBase' => array(),
)

Therefore, we need to execute the array_merge_recursive() first, before checking whether the resulting $modules array is empty.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Why are you removing the FALSE for $enable_dependencies flag?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Hah. Quoting @tstoeckler from #6:

In case this needs a re-roll feel free to remove the second param to enableModules() while we're at it, as that is unused.

Indeed, DUTB::enableModules() does not expose the second argument. That is, because the modules are not actually installed; they're just added to the current module list.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

epic self-#fail. :-)
Thanks for the reminder!

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

I'm aware that this is a follow-up patch, but I think it's just a minor annoyance, so I hope we can move forward without adding a test for this change.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

9: drupal8.test-modules-empty.9.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC, I guess...

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: drupal8.test-modules-empty.9.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Reviewed & tested by the community
FileSize
drupal8.test-modules-empty.22.patch1 KB

Re-rolled for KernelTestBase.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 22: drupal8.test-modules-empty.22.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review

22: drupal8.test-modules-empty.22.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 22: drupal8.test-modules-empty.22.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review

22: drupal8.test-modules-empty.22.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
2207377-psr4-reroll.patch957 bytes

Reroll for #2247991: [May 27] Move all module code from …/lib/Drupal/… to …/src/… for PSR-4.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed c8faf22 and pushed to 8.x. Thanks!

  • Commit c8faf22 on 8.x by alexpott: 
    Issue #2207377 by sun, xjm: Fixed Bogus "Enabled modules: ." assertions...

Status: Fixed » Closed (fixed)

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