This, however, won't fail if the module happens to include default config that is owned by that module.
Discovered in #1779026: Convert Text Formats to Configuration System, where I try to add a filter format for php.module.

CommentFileSizeAuthor
#1 i-hate-enable-disable-test.patch677 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Priority: Normal » Major
Status: Active » Needs review
FileSize
677 bytes

See attached.

Bumping priority since it blocks the correct fix in #1779026: Convert Text Formats to Configuration System (which is major)

xjm’s picture

Status: Needs review » Reviewed & tested by the community

I swear I've made this exact change before, but I can't find the issue. RTBC assuming it passes tests (and @tim.plunkett says it did locally).

xjm’s picture

Issue tags: +VDC
damiankloip’s picture

Yeah, when you see it there it does look a bit silly :)

Would any other test using this ever need to optionally add a prefix? I guess not really.

Looks good!

sun’s picture

Status: Reviewed & tested by the community » Needs review

@xjm's memory is wrong ;) - this patch reverts our recent change from:
#1850158: Bugged assumption in ModuleTestBase::assertModuleConfig()

That issue essentially introduced the module prefix to fix a range of problems down the line.

Now this issue essentially wants to revert it back to fix different problems down the line. (That said, the issue summary needs to be updated to clarify what exact problems we're facing.)

It might make more sense to mark this as duplicate of aforementioned issue and re-open that one.

Ultimately, we seem to be circling back into the epic #1776830: [META-1] Installation and uninstallation of configuration provided by a module that belongs to another module's API here. We can possibly try to work around the larger problem space, although the fact that we're trying to duct-tape the same test assertion line back and forth within less than a month already tells a tale.

tim.plunkett’s picture

Status: Needs review » Closed (duplicate)

This was actually introduced recently, and should just be reverted: #1850158: Bugged assumption in ModuleTestBase::assertModuleConfig()

xjm’s picture