Duplicate BrokenSetUpTest for BrowserTestBase

Just poking around this. The docblock for the test states:

* If a test case does not call parent::setUp(), running
* \Drupal\simpletest\WebTestBase::tearDown() would destroy the main site's
* database tables. Therefore, we ensure that tests which are not set up
* properly are skipped.

Well, that is not true (edit: For BTB at least). I can run a BrowserTestBase with an empty setUp method without problems from the command line and the Simpletest UI.

It also appears to me, that the test is actually doing something with broken tearDown more then broken setup. But I don't get what it's testing completely yet.

Duplicating this 'as is', is pretty hard, also, we would lose the coverage if (when?) we lose the Simpletest UI.

Would a simple Unit test that makes sure we don't call cleanEnvironment if setUp isn't called do the trick?

I think duplicating the test is a great ideal. Let get this in.

Yep this seems like a good test of the situation. Once we have $this->kernel we have installed the test site properly so testing that we never call this method makes sense.

However the new test is extremely brittle because there is no validation that the cleanupEnvironment() method exists.

I suggest changing the test to be something like:

  public function testTearDownWithoutSetUp() {
    $dangerous_method = 'cleanupEnvironment';
    $this->assertTrue(method_exists(BrowserTestBase::class, $dangerous_method));
    $btb = $this->getMockBuilder(BrowserTestBase::class)
      ->disableOriginalConstructor()
      ->setMethods([$dangerous_method])
      ->getMockForAbstractClass();
    $btb->expects($this->never())->method($dangerous_method);
    $ref_tearDown = new \ReflectionMethod($btb, 'tearDown');
    $ref_tearDown->setAccessible(TRUE);
    $ref_tearDown->invoke($btb);
  }

Without the $this->assertTrue(method_exists(BrowserTestBase::class, $dangerous_method)); you can change $dangerous_method to something like foobar and the test will pass. Basically tests without only negative assertions are always a red flag.

@alexpott yeah that makes sense, updated. Also changed the group of BrokenSetUpTest to 'WebTestBase' to follow the pattern in #2932909: Convert web tests to browser tests for Simpletest module

Nice suggestion @alexpott.

Committed and pushed 4da678eb5f to 8.7.x and eb3e150d72 to 8.6.x. Thanks!