Fully deprecate and prepare for removal of SimpleTest module

Given the IS, this is postponed on #3075490: Move simpletest module to contrib

This is two patches. One is for 8.8.x, and one is for 9.x.

The 9.x patch starts with @lendude’s patch from #3075490-38: Move simpletest module to contrib, and then adds hook_requirements().

The 8.8.x patch only adds hook_requirements() to the existing module with some explanation.

Todo: Figure out the language, add CR, link to it from hook_requirements().

Updated the text a bit, and we need to uninstall the simpletest module before running updates in update tests or they will be blocked.

This should clear up some fails.

So we need to rebuild the container inside the test after uninstalling stuff.

Added a CR, https://www.drupal.org/node/3091784, so now we just need to point to that in the requirements message.

Added the ref to the CR.

Just grammar fixes.

Issue is targeted at 8.8.x, but the last patch ran against 8.9.x.

The patch applies to both, and to 9.0.x. I'm pretty sure the D9 part of this issue will require a different patch to remove everything but the hook_requirements() part. That should maybe be a follow-up since its such a different scope?

Adding an 8.8.x test to #19.

I think #3075490: Move simpletest module to contrib covers the scope for the 9.0.x-only patch. This would be committed first prior to the 8.8 beta; that other can be done any time prior to the 9.0 beta.

Thanks to everyone working on this, and especially to those willing to maintain simpletest in contrib!

Patch mostly looks good. A few concerns:

A)

+++ b/core/modules/simpletest/simpletest.install
@@ -22,6 +22,12 @@
+    'value' => t('The Drupal core Simpletest module is deprecated for removal in Drupal 9. You will need to uninstall the module before continuing. You should use the contributed Simpletest module instead. See https://www.drupal.org/node/3091784.'),

Shall we link directly to the project page, too? Or we're intentionally only linking to the CR here?

B) Shouldn't we have some kind of official release for 8.x at https://www.drupal.org/project/simpletest before we do this? Right now, all that's available is 8.x-3.x-dev.

C) (Partially contradicting myself from point A): The simpletest project page could use some help:

1. It says "Minimally maintained"
2. The project is owned by a user named "BR0kEN" (that's funny and all, but a bit scary for people landing here). ;)
3. The project description starts with talk of D6, then works through a few paragraphs of D7 stuff with "Simpletest is now in core!" with the only mention of D8 below the fold. Maybe we can purge the D6 cruft, re-write the D7 stuff, and move the D8 stuff to the top?
4. It doesn't appear that @Lendude is listed as an official maintainer, which makes the "Maintainers for SimpleTest" block misleading. It appears that the last commit was 2 years ago, unless you know to click on the "View commits" link and understand the inner workings of git commit authorship and how that relates to d.o project nodes.
   

D) The CR is a bit confusing / incomplete:

The Simpletest module is only available in Drupal core to allow you to uninstall it. Any update will be blocked if you have the core version of the module installed. You will need to uninstall it before you can continue updating a site.

If you require the Simpletest module you are advised to use the contributed version.

Can we get clear instructions on how to do this? When can I install/enable the contrib version? Since they have the same name, wouldn't installing the contrib module also break updates? Which update(s) are broken/prevented? When/how is it safe to actually re-install the contrib version? etc.

Thanks again,
-Derek

Re: #23:

Re: #22.A: Yup, a level of indirection is often a good thing. ;)

Re: #22B: Hrm, okay, fair enough. But I'd still feel better with *something*, but if it's just the UI test runner, yeah, cool. But I thought it also held the deprecated test base class (I forget what it's called) and some folks might still have a bunch of tests that they can't sink the time/$ into updating. I'm sure they'd appreciate at least an 8.x-3.0-alpha1 or something to target...

Re: #22D: Okay. When it's not 4am (and if no one beats me to it) I'll update the CR with that.

Re: #24: Generally agree with the generalizations. But we lost "You will need to uninstall the module before continuing." which seems like really important, concrete info that needs to be said in the status warning. Can we split the difference?

t('The Drupal core Simpletest module is deprecated for removal in Drupal 9. It should not be enabled on production sites. You will need to uninstall the module before continuing. See https://www.drupal.org/node/3091784 for alternatives.')

?

Thanks,
-Derek

All the hooks provided by the module are already deprecated and use \Drupal::moduleHandler()->invokeAllDeprecated() so any contrib module leveraging them is already getting deprecations.

There's also the simpletest/drupal.simpletest library that we could deprecate. However I think that depending on the module and using the library is a very limited use-case. There are the routes too but I don't think anything will be depending on them.

One thing I've noticed is that this change prevents updating from 8.7.whatever to 8.8.0 (if this patch is applied). I think we should probably take a leaf out of the block_place module's deprecation patch and only run the deprecation requirement at runtime and perhaps consider changing it to a notice as well.

Looks great to me.

#28 and #30 lost the "fix" from #26. Seems accidental, not intentional.

Otherwise, agreed on all points.

Thanks!
-Derek

Since we want to see a clean bot run, anyway, this restores #26 on top of #30

Thanks eagle-eyed @dww, I guess it's fine to get it back to RTBC now :)

Sorry, sleep deprivation. ;) I now understand -- we no longer prevent you from running updates. Duh.

#30 is RTBC after all. Thanks/sorry!

-Derek

p.s. I canceled the test run for #35 on 8.9.x, but there seems to be no way to cancel https://www.drupal.org/pift-ci-job/1465930 since it's still queuing. That'd be really nice to allow...

Yup, thanks! Slowly waking up... :)

Re-hiding my patch + interdiff.

edit: and finally was able to cancel the testbot runs against #35

I agree with #40. I've added the same note from the end of the CR to the release note for now. We can remove it if/when product management does sign off on the main issue.

+++ b/core/modules/simpletest/simpletest.install
@@ -22,6 +22,12 @@
+    'value' => t('The Drupal core Simpletest module is deprecated for removal in Drupal 9. It should not be enabled on production sites. See https://www.drupal.org/node/3091784 for alternatives.'),

So the name of the module in the user interface is actually "Testing". A less looped-in site owner might see "Simpletest" and have no idea what they need to do to resolve the warning.

Also, I don't think the message should make it sound as though there are alternatives for production sites?

We can say "The Testing module (SimpleTest)" or such. (Note casing on "SimpleTest".)

Here is updated working as per #42
and #43 and #45

Here's a patch for this:

t('The Drupal core <em>SimpleTest</em> (SimpleTest) module is deprecated for removal in Drupal 9. It should not be enabled on production sites. See https://www.drupal.org/node/3091784 for alternative ways to run tests during development.')

Does that address all concerns?
Do we want "SimpleTest" in a placeholder so it can't be translated? Or is this better?

Thanks,
-Derek

edit: eeek, x-post. Oh well, I guess committers can pick which wording they prefer. ;)

p.s. I was trying to avoid double "for" -- "for alternatives for running..." which is why I went with "for alternative ways to run..."

#47 repeats "SimpleTest" twice, so let's use #46. @dww, want to RTBC it?

Sorry folks, 4 hours of sleep. I should probably not do *anything* for Drupal today. :/

This fixes the double trouble.

RTBC. Committer discretion for this or #46. I think this is (slightly) better, but I don't trust myself at all right now. ;)

Thanks/sorry,
-Derek

(slightly better title for the commit message)

So there's another problem that I was crossposting: The URL is just stuck in the string as plain text. It won't even be clickable because input filters don't run on translated strings. Even if it were clickable, it's not good accessibility to have a link text be a URL, rather than words meaningful out of context. I'd suggest:

The Drupal core <em>Testing</em> (SimpleTest) module is deprecated for removal in Drupal 9. It should not be enabled on production sites. Read <a href="https://www.drupal.org/node/3091784">The Drupal core SimpleTest module is deprecated</a> for alternative ways to run tests during development.

Explicitly assigning to Ted for less crossposts. ;)

🤞

1. I prefer @xjm's:
Read <a href="https://www.drupal.org/node/3091784">The Drupal core SimpleTest module is deprecated</a>
vs.
See <a href="https://www.drupal.org/node/3091784">the change record about the Testing module\'s deprecation</a>

2. I still prefer:
"... for alternative ways to run"
vs.
"...for alternatives for running"

I don't want to hold up progress, but I don't want to call #52 RTBC, either. :/

Sorry,
-Derek

words are hard
🤞🤞🤞🤞🤞🤞🤞🤞🤞🤞

Looks great, thanks!

Meanwhile, updated the CR again now that the dust is settling in here:
https://www.drupal.org/node/3091784/revisions/view/11637799/11638010

Cheers,
-Derek

Thanks @tedbow and @dww! I committed this without waiting for the final test run, since the same thing with slightly different words passed already, the clock is ticking on the beta release window, we're in a commit freeze anyway, and HEAD tests will prove that this passes.

Marking fixed; we'll reopen or file a followup if the product management decision in #3075490: Move simpletest module to contrib is different from what this issue expects. I'll also publish the CR.

Yay, thanks!

@xjm: Re: "I'll also publish the CR." I already did. ;)

-Derek

Nice! Awesome job all!