Problem/Motivation

  • If you have the minimal profile it is not possible to run Drupal\standard\Tests\StandardTest
  • If you have the standard profile it is not possible to run Drupal\minimal\Tests\MinimalTest
  • Currently testbots are not running Drupal\minimal\Tests\MinimalTest since the test running site has Standard installed

Proposed resolution

This was broken by #2188661: Extension System, Part II: ExtensionDiscovery. We have competing expectations of unused installation profile's module tests not appearing and being able to run tests contained within unused installation profiles. This patch ensures and tests that all tests are runnable with no dependency on the current installation profile.

Remaining tasks

Review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#7 2217781.7.patch4.15 KBalexpott
#7 5-7-interdiff.txt2.06 KBalexpott
#5 2217781.5.patch4.1 KBalexpott
#5 1-5-interdiff.txt3.66 KBalexpott
#1 2217781.1.patch2.31 KBalexpott
#1 2217781.1.will-fail.patch1.28 KBalexpott

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2217781.1.will-fail.patch1.28 KB
new 2217781.1.patch2.31 KB

If we set the profilesDirectory property to an empty array on the ExtensionListing object then it will scan all profiles.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Testing system
Parent issue: » #2188661: Extension System, Part II: ExtensionDiscovery

Well-spotted! The added test assertion looks sufficient and good to me.

I already questioned this when I studied the code in the ExtensionDiscovery patch. But somehow, the previously existing code in HEAD was using drupal_system_listing() — whereas that was a wrapper around SystemListingInfo, which operated on the current list of installation profiles only.

In other words, the ExtensionDiscovery patch performed a straight conversion of the existing test discovery code in simpletest.module. Looks like some other patch wrongly changed that code to use SystemListingInfo in the past already.

The last submitted patch, 1: 2217781.1.will-fail.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 2217781.1.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 1-5-interdiff.txt3.66 KB
new 2217781.5.patch4.1 KB

So it looks like we have competing expectations. OtherInstallationProfileModuleTestsTest is testing that tests in unused installation profiles are not available. I think this expectation is incorrect. We should be able to run all available tests.

The attached patch refactors OtherInstallationProfileModuleTestsTest to meet this expectation and renames it OtherInstallationProfileTestsTest.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+    // Assert the existence of a test for an uninstalled profile.
...
+    // Assert the existence of a test for a module in an uninstalled profile.
#
+      // Ensure that tests in uninstalled profiles can be discovered.
...
+    // Ensure that tests in uninstalled profiles can be discovered.

Hm, you're (at least consistently) using the term "uninstalled" in these comments, but the "install/uninstall" terminology does not really mesh so well with installation profiles.

In simpletest.module, let's change the wording to this:

"Ensure that tests in all profiles are discovered."

In the test, let's change the wording to this:

"Assert the existence of a test in a different installation profile than the current."

(or shorter, if you find a way to shorten it)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 5-7-interdiff.txt2.06 KB
new 2217781.7.patch4.15 KB

Makes sense.

sun’s picture

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

Thanks!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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