Problem

  • Various patches in the queue are or will fail on re-test if they happen to use the Testing profile and if they still happen to assume that Node module gets installed.

Goal

CommentFileSizeAuthor
#7 drupal8.testing-node.7.patch1.79 KBsun
#4 drupal8.testing-node.4.patch950 bytessun
drupal8.testing-node.0.patch343 bytessun

Comments

aspilicious’s picture

aspilicious commented
Status: Needs review » Reviewed & tested by the community

Ok that has to be done :)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

FWIW, I manually tested and can confirm that this change resolves fatal errors and makes such patches pass again; e.g.:
#1376166-28: Custom logo and favicon functionality inanely tries to support absolute local file paths

sun’s picture

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

Hold on, we also need the 'access content' permissions.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new drupal8.testing-node.4.patch950 bytes

Attached patch additionally restores/reverts the testing.install change of the Node-optional patch:
http://drupalcode.org/project/drupal.git/blobdiff/25043d962b6d2bf755d7a1...

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

The permissions are required, because tests, which assume that Node module is installed, also assume that the built-in anonymous and authenticated user roles can 'access content'. Merely installing Node module does not grant the permissions.

The missing permissions also explain the huge amount of test failures in the patch with ".node." suffix in http://drupal.org/node/1373142#comment-5443554 - which should have passed just like the one before.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal8.testing-node.4.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new drupal8.testing-node.7.patch1.79 KB

The reason for why that test fails is that the test already uses the Testing profile, but does not account for (non-required) modules being already installed by the installation profile.

Therefore, the test worked, since the Testing profile did not install any additional modules in the past.

Attached patch makes the test account for already pre-installed modules by the installation profile, so that these are not attempted to be enabled. The final module disable and uninstall assertions are still executed for them though (since they are optional).

Thus, this patch should come back green and be RTBC.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Alright, so the patch passed tests.

However, I'm no longer sure whether we need to do this, because we're in a weird situation now:

  1. #1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths and all patches adding or changing tests using the Testing profile are failing.
  2. But: #1376150: Shortcut module installation fails in tests when installed later (due to menu system not saving menu links correctly) actively tests against the Testing profile without Node module, as that revealed another somewhat major bug.

Kind of a chicken-and-egg situation.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Closed (works as designed)

Grace period is over.