False pass with WebTestBase::assertFieldByName with select element

1. +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
   @@ -0,0 +1,83 @@
   + * Tests helper methods provided by the abstract WebTestBase class.
   + */
   

   Let's add a @group Drupal at least.

2. +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
   @@ -0,0 +1,83 @@
   +class WebTestBaseTest extends UnitTestCase {
   +
   

   HAHAHAHHA

3. +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
   @@ -0,0 +1,83 @@
   +  public function assertFieldByNameProvider() {
   ...
   +   *
   +   * @dataProvider assertFieldByNameProvider
   

   Let's start with provider to avoid some confusion I had.

+++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
@@ -0,0 +1,83 @@
+  /**
+   * Provides data for testing the assertFieldByName() helper.
+   *
+   * @return array
+   *   An array of values passed to the test method.
+   */
+  public function assertFieldByNameProvider() {

Yah, this is super confusing, especially since method names beginning with 'assert' tend to have special meaning as well. ("Wait, what, is that a new assertion being defined??")

Name it providerTestAssertFieldByName() please.

+1 on @group Drupal and also add @group Simpletest.

1. +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
   --- /dev/null
   +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/Fixtures/html/select_2nd_selected
   
   +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/Fixtures/html/select_2nd_selected
   --- /dev/null
   +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/Fixtures/html/select_none_selected
   

   I guess we should name the file with .html ending.

2. +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
   @@ -0,0 +1,84 @@
   +class WebTestBaseTest extends UnitTestCase {
   

   Still mindblowing :P

3. +++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
   @@ -0,0 +1,84 @@
   +
   +  public static function getInfo() {
   

   Let's add a {@inheritdoc} now.

Thank you, that is perfect now.

12: drupal-2105617-12.patch queued for re-testing.

12: drupal-2105617-12.patch queued for re-testing.

12: drupal-2105617-12.patch queued for re-testing.

Re-RTBC

18: drupal-2105617-18.patch queued for re-testing.

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -1280,7 +1280,7 @@ protected function parse() {
-        $this->pass(t('Valid HTML found on "@path"', array('@path' => $this->getUrl())), t('Browser'));
+        $this->pass(String::format('Valid HTML found on "@path"', array('@path' => $this->getUrl())), 'Browser');

Why do we think that we can do this? t() is used all over WebTestBase including in assertFieldByName() just removing one call to it seems inconsistent. I would prefer the original solution to this issue in #9.

Yes but following that link to find the issues you find #1601146: Allow custom assertion messages using predefined placeholders which has a different solution - and one that addresses all usages of t() in WebTestBase - that's why I think just removing one usage is inconsistent.

+++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
@@ -0,0 +1,87 @@
+   * @see \Drupal\simpletest\WebTestBase::assertFieldByName().

Coding standards. Lose the period at the end.

Other than that, RTBC (again). :-)

Committed 20d5b0f and pushed to 8.x. Thanks!

Fixed on commit.

diff --git a/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
index 1cf0600..bbfdadd 100644
--- a/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
+++ b/core/modules/simpletest/tests/Drupal/simpletest/Tests/WebTestBaseTest.php
@@ -57,7 +57,7 @@ public function providerAssertFieldByName() {
    * @param bool $expected
    *   The expected result of the assert.
    *
-   * @see \Drupal\simpletest\WebTestBase::assertFieldByName().
+   * @see \Drupal\simpletest\WebTestBase::assertFieldByName()
    *
    * @dataProvider providerAssertFieldByName
    */

Oops an automated version of me got it wrong

FWIW, this is/was partially a duplicate of #2171939: Invocation of WebTestBase::assertField and WebTestBase::assertNoFieldByName methods with wrong parameters — which discovered many more bogus invocations.