Convert web tests AssertNoPattern to Browser Test

+1 for sure!

+++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
@@ -668,6 +668,22 @@ protected function assertPattern($pattern) {
+   */
+  protected function assertNoPattern($pattern) {
+    @trigger_error('assertNoPattern() is deprecated and scheduled for removal in Drupal 9.0.0. Use $this->assertSession()->responseNotMatches($pattern) instead. See https://www.drupal.org/node/2864262.', E_USER_DEPRECATED);
+    $this->assertSession()->responseNotMatches($pattern);
+  }

Do you mind providing some test coverage for that in \Drupal\Tests\Core\Assert\AssertLegacyTraitTest?

Thanks! I don't understand why we can't use $this->assertSession()->responseNotMatches(), can you explain that again? You just need to mock stuff on $this->webassert. See testAssertOptionSelected() for example.

That test would then just confirm that the call to assertNoPattern() is passed through.

I have followed @klausi's suggestion and it works well for testAssertNoPatternFail(), but not testAssertNoPattern(). Any ideas?

@GoZ I couldn't agree more, but that is often the way with tests and mocks. The example @klausi recommended, testAssertOptionSelected(), is only one small step away from the same thing. I would be interested to hear @klausi's opinion...

@GoZ: you would be testing that assertNoPattern() works as you intend it to work: just passing through stuff to responseNotMatches(). So the unit test confirms that
1) the method assertNoPattern() exists and is callable
2) The calls are forwarded to responseNotMatches()

That is the nature of a unit test: it only tests that little piece that you just wrote.

However, we can also add a test to BrowserTestBaseTest instead if you prefer that?

@klausi if the testbot replicates my local installation then testAssertNoPattern() will fail due to not containing an assert - are you aware how to avoid this?

@Jo: you can do ->shouldBeCalled() or shouldNotBeCalled() on the mock. That will produce an assertion in PHPUnit.

1. +++ b/core/tests/Drupal/Tests/Core/Assert/AssertLegacyTraitTest.php
   @@ -154,6 +154,33 @@ public function testAssertOptionSelectedFail() {
   +    $this->webAssert
   +      ->responseNotMatches('/.*foo$/')
   +      ->willReturn(NULL);
   

   You should add a ->shouldBeCalled() here.

2. +++ b/core/tests/Drupal/Tests/Core/Assert/AssertLegacyTraitTest.php
   @@ -154,6 +154,33 @@ public function testAssertOptionSelectedFail() {
   +  /**
   +   * @covers ::assertNoPattern
   +   */
   +  public function testAssertNoPatternFail() {
   +    $this->page->getContent()->willReturn('foo bar bar');
   +    $this->setExpectedException(PHPUnit_Framework_ExpectationFailedException::class);
   +
   +    $this->webAssert
   +      ->responseNotMatches('/^foo.*$/')
   +      ->willThrow(\PHPUnit_Framework_ExpectationFailedException::class);
   +
   +    $this->assertNoPattern('/^foo.*$/');
   +  }
   

   I think we can remove this test. We only want to test that assertNoPattern() can be called and that it forwards calls to responseNotMatches(). That is covered by the first test already.

+++ b/core/tests/Drupal/Tests/Core/Assert/AssertLegacyTraitTest.php
@@ -154,6 +154,33 @@ public function testAssertOptionSelectedFail() {
+    $this->page->getContent()->willReturn('foo bar bar');

This line can be removed.

We have a test, we have the required documentation ...

Committed and pushed 47d0521 to 8.4.x and fd92fc7 to 8.3.x. Thanks!

Committed to 8.3.x because keeping the test API the same is important.

Updated and published CR.