Deprecation: add $defaultTheme - create FormWidgetBrowserTestBase class

OK, yes. Something like this (only done for TextInputWidgetTest.php in this patch). How were you visualising forcing the actual tests to implement those three standard tests? I have defined them in the base class and set a guaranteed failure assertion.

What's the best way to add Drupal\Core\TypedData\DataDefinition into the base class so that each of the actual test classes do not have to add it? If I add it, it fails phpcs saying 'unused use statement'

OK, thanks for the info. Yes that helper function would be good and I will raise a separate issue for that, which I can work on once this issue is fixed.

Whenever I see a "fix" that needs to be made in half a dozen different classes ...

Yes I absolutely agree, I always think that too. Reducing code duplication helps us all.

Regarding the abstract test class, phpunit just produces the abstruse and not particularly enlightening message

Fatal error: Class Drupal\Tests\typed_data\Functional\TypedDataFormWidget\BrokenWidgetTest contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\Tests\typed_data\Functional\TypedDataFormWidget\FormWidgetBrowserTestBase::testValidation) in /Library/webServer/web/dr-source/modules-D8-git/typed_data/tests/src/Functional/TypedDataFormWidget/BrokenWidgetTest.php on line 17

I do understand the message, but it is not very user-friendly. Implementing those three functions as public, with just a single code line

  public function testValidation() {
    $this->fail('There is no implementation of the mandatory test function: ' . $this->getName());
  }

gives a much more immediately useful message

There was 1 failure:
1) Drupal\Tests\typed_data\Functional\TypedDataFormWidget\BrokenWidgetTest::testValidation
There is no implementation of the mandatory test function: testValidation

I have added a $widgetId protected property to each test class, which then allows the creation of the widget to be done in the Base class setUp() instead

This patch will fail because two widget tests do not implement all of the three mandatory test functions.

Updated patch with those two missing testValidation functions implemented. For TextareaWidgetTest I wrote a proper test using a constraint on length, minimum 40 chars. For BrokenWidgetTest there is a just an empty implementation as there is no validation possible (I think?).

Thanks for the feedback.

... define the base class setUp() function to accept $widgetId as an argument.

I tried that. In the base class protected function setUp(string $widgetId) { gives:

Fatal error: Uncaught Declaration of Drupal\Tests\typed_data\Functional\TypedDataFormWidget\FormWidgetBrowserTestBase::setUp(string $widgetId) should be compatible with Drupal\Tests\BrowserTestBase::setUp()

Hence I removed the parameter definition, and extract it via func_get_arg(). If the parameter is not specified the default error message is
func_get_arg(): Argument 0 not passed to function which is not very friendly, so I have made this more helpful by failig the test with:

Function parent::setUp() must be called with a widget id parameter

I used the condensed syntax

    func_num_args() == 1 ? $this->widgetId = func_get_arg(0)
      : $this->fail('Function parent::setUp() must be called with a widget id parameter');

but let me know if you prefer it written out with a full if .. then .. else

But doesn't that trigger a 'risky test' warning?

No oddly it seems to be fine. My local testing and on Travis are clean, no warnings.

I can't make it work just with the base class protected function setUp($widgetId = 'broken'). That produces the similar error but the other way round:

Fatal error: Uncaught Declaration of Drupal\Tests\typed_data\Functional\TypedDataFormWidget\BrokenWidgetTest::setUp() should be compatible with Drupal\Tests\typed_data\Functional\TypedDataFormWidget\FormWidgetBrowserTestBase::setUp($widgetId = 'broken')

i.e. the sub-class definition protected function setUp() does not match the base class. So now the calling functions have to changed to look like

  protected function setUp($widgetId = 'select') {
    parent::setUp($widgetId);
  }

I tested calling without a parameter so that the widget defaulted to 'broken' but this was very confusing. There was nothing to show what was wrong, just an unexpected set of test failures, e.g. the isApplicable result was wrong. It could be frustrating to try and fix tests when actually you are inadvertantly testing against the wrong widget. So I have defaulted to blank, and force the same helpful test failure message as before.

The problem now is that we get coding standards warnings for each of the sub-class functions:

FILE: ...ta/tests/src/Functional/TypedDataFormWidget/SelectWidgetTest.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 33 | WARNING | Possible useless method overriding detected
    |         | (Generic.CodeAnalysis.UselessOverridingMethod.Found)
----------------------------------------------------------------------

Patch attached, but clearly this still needs some change.

I have created #3110650: Move creation of $widget into WidgetBrowserTestBase and postponed it on this issue. The setUp() functions will remain as-is, so that I can get back to scope and fix the $defaultTheme.

Back to a simple version of the patch, to create the new FormWidgetBrowserTestBase and define $defaultTheme

I have realised that the $modules property should be 'protected' not 'public' so might as well make that correction now. Also the sub-classes only have to define additional modules that are extra to those set in the base class.

Thank you. Unassigning.

Expanded title as this issue creates tests/src/Functional/TypedDataFormWidget/FormWidgetBrowserTestBase.php

Also I have unpostponed #3110650: Move creation of $widget into WidgetBrowserTestBase