Problem/Motivation

We need to convert everything over to BrowserTestBase that needs converting, we need to deprecate everything that isn't already deprecated.

This is the big one, and should probably be postponed until all the other modules have been converted.

As much as possible, we want to leave tests which actually test WTB itself alone. They will eventually be removed as part of the simpletest module deprecation.

Tests which look at the behavior of the UI forms can be moved to functional BTB tests.

Proposed resolution

KernelTestBaseTest is out of place. Added another issue to limit scope here, since it's not completely trivial: #2942633: Move KernelTestBaseTest out of simpletest module

Add @group WebTestBase to tests of WTB so there is no confusion in the future.

Combine Drupal\simpletest\Tests\UiPhpUnitOutputTest and Drupal\simpletest\Tests\SimpleTestBrowserTest::testTestingThroughUI() into
Drupal\Tests\simpletest\Functional\SimpletestUiTest.

Remaining tasks

#2942633: Move KernelTestBaseTest out of simpletest module

User interface changes

API changes

Data model changes

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Lendude created an issue. See original summary.

dawehner’s picture

Given that we still need to maintain the simpletest UI for longer maybe it would make sense to copy these tests. This is just a random though which popped into my head.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Mile23’s picture

After some analysis, here's the result.

First, there's a KTB test hiding in there: #2942633: Move KernelTestBaseTest out of simpletest module

Most of the remaining WTB tests in simpletest module are tests of WTB or the UI form behavior.

Tests where we run tests through the form can be converted to BTB, which we should do because we might need the form into D9, tho likely not. #2750461: Remove Simpletest UI because we don't want to maintain a graphical test runner

The rest of the WTB tests can be left as-is, because the goal is to get rid of WTB. Some of the test class names are ambiguous, and really should be cleaned up so we know what they test, but that should be a very (VERY) low priority. For instance, there's BrowserTest and SimpletestBrowserTest, which cover similar areas, but you couldn't tell what based on the class names.

Attached is a patch which adds @groups to WTB tests and @todos for which tests/methods to move.

Updating IS.

Mile23’s picture

After some work, it turns out that Drupal\simpletest\Tests\UiPhpUnitOutputTest is tricky as a BTB test. It calls simpletest_phpunit_run_command() which exec()s PHPUnit with --printer SimpletestUiPrinter. If you have your phpunit.xml set up to use HtmlOutputPrinter (which you should...), then PHPUnit will use HtmlOutputPrinter instead, failing the test.

So since this test is a WTB test, it bypasses all that by virtue of not running under PHPUnit. So we can't move it to be a BTB test without changing it radically.

But it turns out the reason for the test existing is to make sure the output from the test is filtered to have clickable URLs and so forth. This can be unit tested, so this patch adds the unit test of SimpletestUiPrinter. If that's enough to replace UiPhpUnitOutputTest then we can remove it a future patch here.

The rest is fairly straightforward. Drupal\simpletest\Tests\SimpleTestBrowserTest::testTestingThroughUI() was loading the simpletest UI form twice per each test it ran, so that's about a minute and a half more than it needed. That's fixed.

Mile23’s picture

+++ b/core/tests/Drupal/Tests/Listeners/Legacy/HtmlOutputPrinter.php
@@ -15,7 +15,7 @@ class HtmlOutputPrinter extends \PHPUnit_TextUI_ResultPrinter {
-  public function __construct($out, $verbose, $colors, $debug, $numberOfColumns) {
+  public function __construct($out = null, $verbose = false, $colors = self::COLOR_DEFAULT, $debug = false, $numberOfColumns = 80) {

Oh yeah, there's one other thing.

HtmlOutputPrinter (the Legacy one) didn't have the same constructor signature as the superclass.

This woudn't matter for testing SimpletestUiPrinter if we just mocked the class and bypassed the constructor. However, if you look at HtmlOutputPrinter (the not-Legacy one), you'll see that we use a class_alias() to switch the namespace of the class before it's defined.

I think this confuses the PHPUnit mocking system, because I was unable to mock SimpletestUiPrinter, which inherits from one of these two classes, depending.

So the easy solution is to just add all the constructor defaults, copied from the superclass and new one into being.

Mile23’s picture

Fixed CS and adds @covers annotation.

The question still remains from #5: We could just remove the UI test for the output printer, if we're OK with relying on the unit test. No harm in leaving it in, since it doesn't actually spend 20 seconds generating the test form or any such.