Deprecate WebTestBase and its required classes in favour of BrowserTestBase

I would say totally +1 as of 8.4.x
IMHO we should wait until we commit the conversions.

One important point maybe, what should happen with new tests. For me this will be solved kind of automatically and we don't need a policy. People will use the new test base over time, while we convert remaining ones at the same time.

I love it when a plan comes together
+1000

Hm. I think we should fix this #2803621: Break BrowserTestBase & children dependency on Simpletest, deprecate stub BC traits before.

EDIT: Note that BTB is a core feature that depends on a module. This is conceptually wrong.

I think deprecating WebTestBase in 8.3.x is a bit too early. We need to get our big initial conversion patch in, then do some more manual conversions of what is left to gain more experience and confidence in BrowserTestBase. If everything goes smoothly we can then deprecate WebTestBase in 8.4.x, meaning contrib developers will see it first when 8.4.0 is released in October 2017.

I don't think issues such as #2803621: Break BrowserTestBase & children dependency on Simpletest, deprecate stub BC traits are blockers for the deprecation because BrowserTestBase is usable without that.

Postponing this at least on #2770921: Feb 21st: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits. I think we need a bigger chunk converted to BrowserTestBase before we deprecate Simpletest.

#2770921: Feb 21st: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits went in, let's continue discussing a timeline.

Our original proposal for a timeline that we came up with in September 2016 in #2807237: PHPUnit initiative: We will deprecate Simpletest's WebTestBase in 8.4.x, but not for 8.3.x releases. Between the release of 8.3.0 and 8.4.0 we will polish our PHPUnit browser tests and then WebTestBase will be deprecated and forbidden for new tests with the release of 8.4.0 in October 2017.

Drupal 8.0.1 is released, so this is back on, please. :-)

Looking for but can't find the how-to-convert-your-test doc. That should be a @see on this.

#2876145: ConfigTranslationViewListUiTest is a WTB test in the Functional namespace adds @trigger_error() for all WTB test base classes.

We might end up undoing that here to avoid double errors.

We might end up undoing that here to avoid double errors.

That is something we could do indeed. I think a duplication is not really a big issue, especially because contrib might do the same as well, but yeah its a good idea to have indeed.

So based on #10 this is essentially postponed on #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

We've got a helpful dataviz at http://simpletest-countdown.org/

We want to be sure to deprecate whatever refactoring happens in #2969741: Deprecate simpletest within DrupalKernel (DRUPAL_TEST_IN_CHILD_SITE)

Re-opening based on #2735005-277: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

Patch still applies. :-)

+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -28,6 +28,9 @@
+ * @deprecated in Drupal 8.4.0, will be removed before Drupal 9.0.0.

Update to 8.7.0.

Also, the IS says we should @trigger_error() in a follow-up, but the linked issue is fixed. We might also consider never adding @trigger_error() because it might be very disruptive.

We are so close now:

find core/modules -print | grep "src/Tests" | grep "Test.php" | grep -v simpletest
core/modules/system/src/Tests/Session/SessionHttpsTest.php
core/modules/file/src/Tests/FileFieldWidgetTest.php
core/modules/field_ui/src/Tests/ManageDisplayTest.php
core/modules/quickedit/src/Tests/QuickEditAutocompleteTermTest.php
core/modules/quickedit/src/Tests/QuickEditLoadingTest.php

Still needs CR. This probably slots in to a bunch of them.

WebTestBase uses two deprecated traits:

• Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait
• Drupal\simpletest\AssertContentTrait

AssertContentTrait has a drop-in replacement so let's switch WTB to use Drupal\KernelTests\AssertContentTrait instead, and then add @trigger_error() to the deprecated one.

I think AssertPageCacheContextsAndTagsTrait can't be replaced easily so we should just leave that without changes.

Updating the title to reflect the motivation.

Classes yes but not traits see #3031580: Undeprecate \Drupal\FunctionalTests\AssertLegacyTrait and \Drupal\KernelTests\AssertLegacyTrait in Drupal 8.

@jibran Hmm I think we need to deprecate all traits too if they live inside the Simpletest module. And if they are needed outside of this, we need to deprecate the version inside and copy it to somewhere outside the module. Does that make sense?

The class @alexpott pointed out on slack is \Drupal\simpletest\TestBase that also needs deprecating here.

Added the deprecation to TestBase as well.

Tried adding a @trigger_error() to TestBase::__construct() but I think simpletest only seems to support showing them when thrown the UI (or maybe while running the test might work too, at the end of setUp()?), so I don't think we need to bother with that. the @deprecated will be enough for phpstan based tools. The advantage is that we don't need any trickery to still run our old simpletest-testing tests.

And yes, we definitely need to deprecate that trait with a @trigger_error(), the green test shows that it works. The use in WebTestBase is necessary, as running tests that run tests through the UI will trigger it, while e.g. kernel tests don't, so we can leave it there. #3031580: Undeprecate \Drupal\FunctionalTests\AssertLegacyTrait and \Drupal\KernelTests\AssertLegacyTrait in Drupal 8 is a very different case, this is a simple update-namespace deprecation.

I think this would be great to get in asap, as it will paint a much more realistic picture for contrib.

+1 to adding the @deprecated - it's a step forward and we can work out the @trigger_error later if we need to.

+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -15,6 +15,7 @@
+use Drupal\KernelTests\AssertContentTrait;

There's a class with the same name in the same folder as WebTestBase. I've seen this go wrong in different environments - let's add an as Something here.

+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -15,6 +15,7 @@
+use Drupal\KernelTests\AssertContentTrait;

The Phpunit AssertContentTrait and Simpletest AssertContentTrait aren't identical (or might not be in the future), so I'm hesitant to do this, it might break stuff in contrib. With only 8 simpletests remaining in core, core is a pretty bad sample for testing if we can do this without consequence.

So I think we should decide what to do with @deprecated code using other @deprecated code, since it will all be removed at the same time, there is no reason to clean it up, other then to get phpstan to pass while it is still in.

It's not just phpstan*, the @trigger_error() do cause the Simletest-UI tests to fail because through that, they get deprecation messages. So we would have to ignore that, but that's pointless.

I'd say we just don't add @trigger_error() then and keep using the old trait and only to @deprecated on all of them.

Hah! Nevermind! They are the same!

<?php

namespace Drupal\simpletest;

use Drupal\KernelTests\AssertContentTrait as CoreAssertContentTrait;

/**
 * Provides test methods to assert content.
 *
 * @deprecated in Drupal 8.6.0, to be removed before Drupal 9.0.0. Use
 *   Drupal\KernelTests\AssertContentTrait instead.
 *
 * @see https://www.drupal.org/node/2943146
 */
trait AssertContentTrait {

  use CoreAssertContentTrait;

}

Sorry about the noise, here is #34 addressed. Ignore #36 since they are identical anyway.

Took the 'as' from the deprecated version of AssertContentTrait to keep the reference to that consistent.

This looks good to add the actual deprecations and if we want to make tests fail too we can add that in a follow-up.

Committed a84d05f and pushed to 8.8.x. Thanks!

Credited all the reviewers.

We need to update:

• https://www.drupal.org/docs/8/testing/converting-d7-simpletests-to-drupal-8
• https://www.drupal.org/docs/8/creating-custom-modules/testing
• https://www.drupal.org/docs/develop/coding-standards/drupal-simpletest-c...

I've fixed up - if people could look at them and improve them or fix where I've messed up that'd be great.

For the coding standards there's #2057905: [policy, no patch] Discuss the standards for phpunit based tests so we've only been trying to fix that for 5 years.

Hmm, I agree with getting the deprecation message on there as soon as possible, but I'm not sure we actually want to remove SimpleTest before Drupal 9. This is at odds with the notion of making the upgrade as smooth as possible. People need their test suites to work in order to test their upgrades, and converting from SimpleTest to PHPUnit-based tests (BTB etc.) is no simple task, as illustrated by how many years it took us to finish this undertaking for core.

Plus, does this change mean all contrib SimpleTests just started failing with a deprecation error? Or have we worked around that on CI yet?

I'm of the opinion that we may need to continue supporting these until Drupal 10. We might come up with a strategy that involves moving SimpleTest to contrib and having CI automatically install it for modules that have SimpleTests. Or something. Let's at least file a followup issue for that. If we can't make some approach like that work, I think we should change this to be removed before 10.0.0 instead.

As mentioned on slack, we only add a @deprecated here, so it only shows up for tools like drupal-check and phpstorm. And we don't show any deprecation errors on drupalCI by default, for now anyway.

Yes, this is likely the most complicated part of updating to D9 for modules that do have tests but I'm +1 on removing it, even though I have a considerable amount of work ahead on porting tests on many of my modules. Regular browser tests are usually fairly easy, only ajax/js tests are annoying because you have to rewrite most of the tests.

The thing about adding this message is that it will start to show up with drupal-check and I think that is very good. Right now, most contributors focus on trivial replacements that I as a maintainer could likely to in as much time as it takes me to review issues and support them. Converting tests is something that will actually help maintainers IMHO.

Thanks @Berdir; I overlooked that there was no @trigger_error() as per earlier comments. Helping drive conversions is good, but I still think we need to tread carefully. I'd feel differently if we'd been ready to do this deprecation even a year ago. :)

@catch and I are discussing a critical followup for the D9 roadmap, which will probably include some but not all of the scope of #2866082: [Plan] Roadmap for Simpletest.

Here's that followup: #3057420: [meta] How to deprecate Simpletest with minimal disruption

We'll want to mention things related to SimpleTest deprecations in the release notes, so let's tag all the related issues for said release notes.

Just polishing the release note a little to use words for the link text (for accessibility).

And polishing it a bit to identify module developers as the target audience.