Make @dataProvider methods static

Working on it!!

Made the changes
Please review!!

The methods that need to be changed are not the ones that have a @dataProvider annotation. It is the methods named in the @dataProvider annotation. See the phpstan reports for the current 6.3.x branch. Also, making a method static requires removing all use of $this inside the method.

@divyansh.gupta: Just to clarify further: It is not only removing usages of $this, but refactoring to maintain the same functionality.

The WebformBreadcrumbBuilderTest @dataProvider is now a static method

There are still several test failures in the latest test-run, will those all be fixed in this issue or in several other issues?
I would suggest doing it all in this issue, since that would reduce the number of needed rebases for the other patches in the queue

It would be great to be everything passing and I'm OK with doing it all in this issue.

Can someone explain the adding of #[LegacyModuleImplementsAlter]? Is this completing work from another issue?

Test are now passing on 6.3.x on Drupal 10. Now let's get them to pass on Drupal 11.

Below are the remaining broken tests. I am going to take a crack at this now.

[0m[31m  41.069s Drupal\Tests\webform\Functional\Element\WebformElementOtherTest           1 passed, 1 failed, 1 log(s)
[0m[31m  27.655s …_export_import\Functional\WebformSubmissionImportExportFunctionalTest    1 passed, 1 failed, 1 log(s)
[0m[31m  10.591s Drupal\Tests\webform\Functional\Cache\WebformCacheTest                    0 passed, 1 failed, 1 log(s)
[0m[31m  10.084s Drupal\Tests\webform\Functional\Composite\WebformCompositeTest            0 passed, 1 failed, 1 log(s)
[0m[31m   9.256s Drupal\Tests\webform\Functional\Element\WebformElementCodeMirrorTest      0 passed, 1 failed, 1 log(s)
[0m[31m   9.527s Drupal\Tests\webform\Functional\Element\WebformElementCounterTest         0 passed, 1 failed, 1 log(s)
[0m[31m   9.699s Drupal\Tests\webform\Functional\Element\WebformElementDateTimeTest        0 passed, 1 failed, 1 log(s)
[0m[31m  12.097s Drupal\Tests\webform\Functional\Element\WebformElementHtmlEditorTest      0 passed, 1 failed, 1 log(s)
[0m[31m  12.512s Drupal\Tests\webform\Functional\Element\WebformElementMediaFileTest       0 passed, 1 failed, 1 log(s)
[0m[31m  11.656s Drupal\Tests\webform\Functional\Element\WebformElementMessageTest         0 passed, 1 failed, 1 log(s)
[0m[31m  12.405s Drupal\Tests\webform\Functional\Element\WebformElementRatingTest          0 passed, 1 failed, 1 log(s)
[0m[31m  11.934s Drupal\Tests\webform\Functional\Element\WebformElementReadonlyTest        0 passed, 1 failed, 1 log(s)
[0m[31m  11.695s Drupal\Tests\webform\Functional\Element\WebformElementSectionTest         0 passed, 1 failed, 1 log(s)
[0m[31m  11.139s Drupal\Tests\webform\Functional\Element\WebformElementStatesTest          0 passed, 1 failed, 1 log(s)
[0m[31m  10.458s …pal\Tests\webform\Functional\Element\WebformElementTermsOfServiceTest    0 passed, 1 failed, 1 log(s)
[0m[31m   9.132s …l\Tests\webform\Functional\Element\WebformElementValidateRequiredTest    0 passed, 1 failed, 1 log(s)
[0m[31m  14.917s Drupal\Tests\webform\Functional\Paragraphs\WebformParagraphsTest          0 passed, 1 failed, 1 log(s)
[0m[31m  43.452s Drupal\Tests\webform\Functional\States\WebformStatesServerTest            0 passed, 1 failed, 1 log(s)
[0m[31m  10.361s …ple_element_properties\Functional\WebformExampleElementPropertiesTest    0 passed, 1 failed, 1 log(s)
[0m[31m  10.030s …l\Tests\webform_image_select\Functional\WebformImageSelectElementTest    0 passed, 1 failed, 1 log(s)
[0m[31m  17.081s …ests\webform_entity_print\Functional\WebformEntityPrintFunctionalTest    0 passed, 1 failed, 1 log(s)
[0m[31m   6.607s Drupal\Tests\webform_node\Functional\WebformNodeTranslationTest           0 passed, 1 errored, 1 log(s)
[0m[31m  10.281s Drupal\Tests\webform_schema\Functional\WebformSchemaTest                  0 passed, 1 errored, 1 log(s)
[0m[31m  17.615s …al\Tests\webform_scheduled_email\Functional\WebformScheduledEmailTest    0 passed, 1 failed, 1 log(s)

A bunch of the tests are broken due to changes in the rendered output, and the few remaining broken tests are a bit trickier.

Fixed

• Drupal\Tests\webform\Functional\Element\WebformElementOtherTest - Removed aria-required="true" attribute
• Drupal\Tests\webform\Functional\Element\WebformElementCodeMirrorTest - Added resize-vertical to textarea
• Drupal\Tests\webform\Functional\Element\WebformElementCounterTest - Added resize-vertical to textarea
• Drupal\Tests\webform\Functional\Element\WebformElementHtmlEditorTest - Added resize-vertical to textarea
• Drupal\Tests\webform\Functional\Element\WebformElementMediaFileTest - Changed image_file_jpg_modal.jpg.webp to image_file_jpg_modal.jpg.avif
• Drupal\Tests\webform\Functional\Element\WebformElementMessageTest - Change attribute order
• Drupal\Tests\webform\Functional\Element\WebformElementRatingTest- Removed aria-required="true" attribute
• Drupal\Tests\webform\Functional\Element\WebformElementReadonlyTest - Added resize-vertical to textarea
• Drupal\Tests\webform\Functional\Element\WebformElementSectionTest - Removed aria-required="true" attribute
• Drupal\Tests\webform\Functional\Element\WebformElementStatesTest- Added resize-vertical to textarea
• Drupal\Tests\webform\Functional\Element\WebformElementTermsOfServiceTest - Removed aria-required="true" attribute
• Drupal\Tests\webform\Functional\Element\WebformElementValidateRequiredTest - Removed aria-required="true" attribute
• Drupal\Tests\webform_submission_export_import\Functional\WebformSubmissionImportExportFunctionalTest - Ignore the changed property.
• Drupal\Tests\webform\Functional\Composite\WebformCompositeTest - Removed aria-required="true" attribute
• Drupal\Tests\webform_example_element_properties\Functional\WebformExampleElementPropertiesTest - Removed aria-required="true" attribute
• Drupal\Tests\webform_image_select\Functional\WebformImageSelectElementTest - Removed aria-required="true" attribute
• Drupal\Tests\webform_schema\Functional\WebformSchemaTest - Adjust rendering
• Drupal\Tests\webform\Functional\States\WebformStatesServerTest - Removed aria-required="true" attribute
• Drupal\Tests\webform\Functional\Paragraphs\WebformParagraphsTest - Change entity comparison to uuid comparison
• Drupal\Tests\webform_entity_print\Functional\WebformEntityPrintFunctionalTest - Attribute order changed

TDB

• Drupal\Tests\webform\Functional\Element\WebformElementDateTimeTest - Still have validation issues
• Drupal\Tests\webform\Functional\Cache\WebformCacheTest - Needs work
• Drupal\Tests\webform_scheduled_email\Functional\WebformScheduledEmailTest - Needs work
• Drupal\Tests\webform_node\Functional\WebformNodeTranslationTest - webform_node_test_translation.module won't install

Fixed

• Drupal\Tests\webform\Functional\Element\WebformElementDateTimeTest - Set date format to 'none' if the date element is set to 'none'.
• Drupal\Tests\webform\Functional\Cache\WebformCacheTest - Needed to recreate webform submission
• Drupal\Tests\webform_node\Functional\WebformNodeTranslationTest - Needed container_rebuild_required: true in *.info.

Needs work

• Drupal\Tests\webform_scheduled_email\Functional\WebformScheduledEmailTest

Fixed

• Drupal\Tests\webform_scheduled_email\Functional\WebformScheduledEmailTest - Save $webform_schedule

Hmm... I am seeing random test failures with the below errors, which I think is more of a CI/CD issue.

[4mWebform Element Ignored Properties (Drupal\Tests\webform\Functional\Element\WebformElementIgnoredProperties)[0m
    [33m ✘ [0mIgnored properties
       [33m┐[0m
       [33m├[0m [43;30mUnexpectedValueException: RecursiveDirectoryIterator::__construct(/builds/project/webform/web/sites/simpletest/45046828/files/php/twig/68a3d5231cd3b_form-element.html.twig_GxylLNVWITg5aAeazUkhb70Op): Failed to open directory: No such file or directory[0m
       [33m│[0m
       [33m│[0m [2m/[22mbuilds[2m/[22mproject[2m/[22mwebform[2m/[22mweb[2m/[22mcore[2m/[22mlib[2m/[22mDrupal[2m/[22mCore[2m/[22mHook[2m/[22mHookCollectorPass.php[2m:[22m[34m513[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/Hook/HookCollectorPass.php[22m[2m:[22m[34m463[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/Hook/HookCollectorPass.php[22m[2m:[22m[34m375[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/Hook/HookCollectorPass.php[22m[2m:[22m[34m137[0m
       [33m│[0m [2m/builds/project/webform/[22mvendor[2m/[22msymfony[2m/[22mdependency-injection[2m/[22mCompiler[2m/[22mCompiler.php[2m:[22m[34m73[0m
       [33m│[0m [2m/builds/project/webform/vendor/symfony/dependency-injection/[22mContainerBuilder.php[2m:[22m[34m814[0m
       [33m│[0m [2m/builds/project/webform/[22mweb[2m/[22mcore[2m/[22mlib[2m/[22mDrupal[2m/[22mCore[2m/[22mDrupalKernel.php[2m:[22m[34m1399[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/DrupalKernel.php[22m[2m:[22m[34m915[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/DrupalKernel.php[22m[2m:[22m[34m828[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/[22mExtension[2m/[22mModuleInstaller.php[2m:[22m[34m729[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php[22m[2m:[22m[34m320[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php[22m[2m:[22m[34m229[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/[22mProxyClass[2m/[22mExtension[2m/[22mModuleInstaller.php[2m:[22m[34m83[0m
       [33m│[0m [2m/builds/project/webform/web/core/lib/Drupal/Core/[22mTest[2m/[22mFunctionalTestSetupTrait.php[2m:[22m[34m511[0m
       [33m│[0m [2m/builds/project/webform/web/core/[22mtests[2m/Drupal/[22mTests[2m/[22mBrowserTestBase.php[2m:[22m[34m537[0m
       [33m│[0m [2m/builds/project/webform/web/core/tests/Drupal/Tests/BrowserTestBase.php[22m[2m:[22m[34m343[0m
       [33m│[0m [2m/builds/project/

Woot! All the tests are passing without any major code changes.

https://git.drupalcode.org/project/webform/-/pipelines/576493

I think we should merge this and move on to preparing for stable D11 release.

Fantastic!

At lot the changes could avoid having a different test in Drupal 10 and 11 if they used XPath. That allows the test to test for the needed elements and attributes without checking for the exact markup.

Is adding LegacyModuleImplementsAlter related to 📌 Convert to OOP Hooks Active ? Perhaps it should have its own commit attached to that issue. That change also introduces a phpstan error. It should have an exception added.

At lot the changes could avoid having a different test in Drupal 10 and 11 if they used XPath. That allows the test to test for the needed elements and attributes without checking for the exact markup.

I am open to any MR that converts HTML checks to XPath.

Is adding LegacyModuleImplementsAlter related to #3487957: Discuss process for converting webform to OOP Hooks? Perhaps it should have its own commit attached to that issue. That change also introduces a phpstan error. It should have an exception added.

I added use Drupal\Core\Hook\Attribute\LegacyModuleImplementsAlter; to webform.module. Let's see if PHPStan is now okay.

I had no idea the conversion to XPath was that simple.

In PHPStorm, I was able to prompt AI to "Convert to `responseContains` to `elementExists` with XPath" for a single assertion and it worked, but I don't have AI support setup for the whole webform code base yet.

@lkmorlan Do as much work as you feel comfortable and if the tests are passing you can merge this at anytime.

I did that one manually. Even manually is pretty simple. I hadn't thought it trying a LLM on it.

phpstan is happy with LegacyModuleImplementsAlter now (though not on Drupal 10). Is that addition a follow-up to 📌 Convert to OOP Hooks Active ? If so, I'll can move that into a separate commit.

I did not add the `LegacyModuleImplementsAlter`, but it seems to be okay. Feel free to move it to a separate commit.

@liam, could we please merge this sooner rather than later, because we will need to update a lot of existing MRs to ensure all the tests are passing?

Tests are now passing on 6.3.x. Thanks all!

I'm not super familiar with the new contribution records yet, but it seems like no one has gotten credit for this issue yet?

@borisson_ Thank you for the nudge to assign credits