Problem/Motivation

There is a need to make the tests in the 8.x-3.x branch pass, so that we can offer consistency to WTB users in Drupal 9.

This is a chicken/egg type thing, because we can't know that this module will support the simpletest framework until it passes tests in this contrib project, but we also have a problem where the core version of simpletest is still present in core.

Proposed resolution

Make the tests pass.

To solve this conundrum, I've forked this module at https://github.com/paul-m/simpletest, and exposed it to Composer at https://packagist.org/packages/mile23/simpletest

The patch in #3075490-44: Move simpletest module to contrib removes (most of) the simpletest module from core, but still allows users to run simpletest-based tests through run-tests.sh.

Combining these two things, we can patch core to remove the module, then use Composer to require mile23/simpletest@dev, and then run the tests through run-tests.sh using group simpletest or --directory modules/contrib/simpletest. (Alternately, clone mile23/simpletest into modules/contrib/ and then set up a path repo in the root composer.json to allow for working on the repo.)

Using this technique, we can develop a patch that begins to solve the problems of running the tests.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 3093460-13.patch4.69 KBlendude
#11 3093460_11.patch2.64 KBmile23
#10 3093460_10.patch1.95 KBmile23
#8 3093460-8.patch3.09 KBlendude
#7 3093460-7.patch3.06 KBlendude
#2 3093460_2.patch1.95 KBmile23

Comments

Mile23 created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 3093460_2.patch1.95 KB

Initial patch which passes tests locally.

However:

+++ b/tests/src/Unit/SimpletestPhpunitRunCommandTest.php
@@ -50,14 +50,17 @@ class SimpletestPhpunitRunCommandTest extends TestCase {
+    // @todo Assumes the module is in modules/contrib/simpletest. There is not a
+    //       good way to populate self::$root (or DRUPAL_ROOT) while this is a
+    //       \PHPUnit\Framework\TestCase subclass.
+    self::$root = realpath(__DIR__ . '/../../../../../../');

Clearly this needs some attention.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

Thanks @Mile23 for starting this up!

I've enabled testing and since the contrib version of a module should take priority over a core version of the same-name module, it should just use the contrib version when enabling 'simpletest'. Just curious to see how hard this fails :)

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

08:52:40 - Installing drupal/simpletest-simpletest (dev-3.x b3d91de): Cloning b3d91de597
08:52:41 Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
08:52:41 Writing lock file
08:52:41 Generating autoload files
08:52:41 > Drupal\Core\Composer\Composer::preAutoloadDump
08:52:41 Hardening vendor directory with .htaccess and web.config files.
08:52:41 Cleaning vendor directory.
08:52:41 > Drupal\Composer\Composer::generateMetapackages
08:52:41 Updated metapackage file composer/Metapackage/CoreRecommended/composer.json.
08:52:41 If you make a patch, ensure that the files above are included.
08:52:41 > Drupal\Composer\Composer::ensureBehatDriverVersions
08:52:41 ---------------- Finished composer_contrib in 20.508 seconds ----------------
08:52:41 ---------------- Starting fetch ----------------
08:52:41 Directory created at /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/ancillary/fetch
08:52:41 Fetch of https://www.drupal.org/files/issues/2019-11-09/3093460_2.patch to /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/ancillary/3093460_2.patch complete.
08:52:41 ---------------- Finished fetch in 0.132 seconds ----------------
08:52:41 ---------------- Starting patch ----------------
08:52:41 Directory created at /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/ancillary/patch
08:52:41 Patch Error
08:52:41 The target patch directory /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/source/modules/contrib/simpletest is invalid.

Running composer require drupal/simpletest-simpletest puts the module in modules/contrib/simpletest-simpletest, so this would require some trickery to get to work I think. That should probably get its own issue.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Also, it's checking out core 8.9:

07:52:12 ----------------   Starting checkout_core   ----------------
07:52:12 Directory created at /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/ancillary/checkout_core
07:52:12 Checking out git repos.
07:52:12 Entering setup_checkout_git().
07:52:12 Performing git checkout of https://git.drupalcode.org/project/drupal.git -b 8.9.x to /var/lib/drupalci/workspace/jenkins-drupal8_contrib_patches-11488/source.

One of the unanswered questions is whether this contrib should work with 8.x or just 9.x. I think it should be 9.x-only, since (AFAIK) the core module will still have WTB through 8.9.x.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

Opened #3093620: When installing with composer the module is installed in the simpletest-simpletest folder to look at the install folder issue.

The 'when to remove from core' question is still up in the air as far as I know, but it does make sense to target D9 for now, so changed the automated testing settings to use D9 for now.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new 3093460-7.patch3.06 KB

Can we simlink something?

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new 3093460-8.patch3.09 KB

Can we do it earlier?

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 3093460_10.patch1.95 KB

Drupal 9 run-tests.sh/drupalci.yml still discovers and runs the Simpletest suite, so we don't need to add a drupalci.yml file. At least, that's the assumption... let's see what actually happens.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 3093460_11.patch2.64 KB

Removes the composer.json file but I bet the Drupal composer facade won't be able to deal with that in just a patch.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

#7-#8 were about trying to simlink the simpletest-simpletest dir to the simpletest dir and seeing if that allowed us to apply patches, but the timing is wrong to start with, so no that didn't work

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new 3093460-13.patch4.69 KB

Lets fix some fatals...

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

The fact that WebTestBase fatals on D9.5 makes me pretty convinced nobody is running Simpletests on Drupal 9.......

Status: Needs review » Needs work

The last submitted patch, 13: 3093460-13.patch, failed testing. View results

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

OMG they actually ran.....