Replace usages of AssertLegacyTrait::assert(No)FieldByXPath, that is deprecated

Adding the "Deprecated assertions" tag to add this into the kanban board https://contribkanban.com/board/Deprecatedassertions

Working on failed tests

Hi, I found the use of $this->xpath() was not correct.
Also, $this->assertNoFieldByXPath is deprecated but still used ion patch.
So solved the issues and uploading the patch.

Missed out a few functions descriptions. Adding the patch and interdiff.

patch isn't applying

Simple reroll

Fixed some of below errors:

1. DOMXPath::query(): Invalid predicate
2. TypeError: Argument 2 passed to Drupal\Tests\BrowserTestBase::xpath() must be of the type array, string given, called in

Still WIP. Will take a look into the failed ones after one more pass

1. +++ b/core/modules/block/tests/src/Functional/BlockHtmlTest.php
   @@ -45,8 +48,8 @@ public function testHtml() {
   +    $this->assertTrue(!empty($elements), 'HTML ID and attributes for test block are valid and on the same DOM element');
   
   +++ b/core/modules/block/tests/src/Functional/BlockTest.php
   @@ -353,7 +353,7 @@ public function moveBlockToRegion(array $block, $region) {
   +    $this->assertTrue(!empty($this->xpath($xpath, [])), new FormattableMarkup('Block found in %region_name region.', ['%region_name' => Html::getClass($region)]));
   

   This should be ->assertNotEmpty(), not ->assertTrue(!empty()). (There are many of these, and probably similar ones.)

2. +++ b/core/modules/block_content/tests/src/Functional/BlockContentCreationTest.php
   @@ -65,7 +65,7 @@ public function testBlockContentCreation() {
   -    $this->assertNoFieldByXPath('//select[@name="settings[view_mode]"]', NULL, 'View mode setting hidden because only one exists');
   +    $this->assertCount(1, $this->xpath('//select[@name="settings[view_mode]"]', []), 'View mode setting hidden because only one exists');
   

   This one looks definitely incorrect. The assertion is turned from asserting that the field does not exist to asserting that the field does exist.

(Note, this was already in the patch before the most recent version, since #8)

Rerolled patch. and have fixed #17.2. Working on fixing occurances of #17.1

Fixed some more failing tests .

Working on it.

Please review the patch.
Thanks

Fixed PHPLint errors.
Please ignore last path and review this one.
Thanks

Fixed failing test cases.
Please review the patch.
Thanks

Fixed remaining failed test cases.
Please review the patch.
Thanks

All occurrences of $this->assertFieldByXPath() and $this->assertNoFieldByXPath() have been replaced.
The deprecation message suppression has been removed.
Added the @expectedDeprecation to the docblock of testXpathAsserts

Please review the patch.
Thanks

Tests are failing since assertFieldByName() uses AssertLegacyTrait::assertFieldByXPath. We will have to wait till https://www.drupal.org/project/drupal/issues/3139406 lands.

Oops, forget to unassign this issue.

Fixing PHPLint error

It needs reroll.
I'll do it.

Please review.

A few places where things can be improved but overall this is looking good, great effort everyone - it's tricky getting all these right.

1. +++ b/core/modules/config/tests/src/Functional/ConfigSingleImportExportTest.php
   @@ -243,8 +243,10 @@ public function testImportSimpleConfiguration() {
   +    $this->assertTrue($this->assertSession()->optionExists("config_type", 'Simple configuration')->isSelected());
   

   Worth breaking this out into two lines?

   $option = ...
   $this->assertTrue($option->isSelected());
   

2. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationSettingsTest.php
   @@ -296,7 +296,7 @@ public function testFieldTranslatableSettingsUI() {
   -    $this->assertFieldByXPath('//input[@id="edit-translatable" and not(@disabled) and @checked="checked"]');
   +    $this->assertSession()->checkboxChecked('edit-translatable');
   

   The not(@disabled) check is lost here.

3. +++ b/core/modules/field/tests/src/Functional/EntityReference/EntityReferenceAutoCreateTest.php
   @@ -105,7 +105,7 @@ protected function setUp(): void {
   -    $this->assertFieldByXPath('//input[@id="edit-test-field-0-target-id" and contains(@class, "form-autocomplete")]', NULL, 'The autocomplete input element appears.');
   +    $this->assertSession()->elementExists('xpath', '//input[@id="edit-test-field-0-target-id" and contains(@class, "form-autocomplete")]');
   

   Here I think we can use fieldExists() then hasClass() separately.

4. +++ b/core/modules/field/tests/src/FunctionalJavascript/EntityReference/EntityReferenceAdminTest.php
   @@ -162,7 +162,7 @@ public function testFieldAdminHandler() {
   +    $this->assertSession()->elementExists('xpath', '//table[@id="field-overview"]//tr[@id="field-test"]/td[1 and text() = "Test"]');
   

   This can be elementContains() for the text part.

5. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
   @@ -635,7 +635,7 @@ public function testHiddenFields() {
   +    $this->assertSession()->elementExists('xpath', '//table[@id="field-overview"]//tr[@id="hidden-test-field"]//td[1 and text() = "' . $field['label'] . '"]');
   

   Same

6. +++ b/core/modules/field_ui/tests/src/Traits/FieldUiTestTrait.php
   @@ -58,7 +58,7 @@ public function fieldUIAddNewField($bundle_path, $field_name, $label = NULL, $fi
   +    $this->assertSession()->elementExists('xpath', "//table[@id=\"field-overview\"]//tr/td[1 and text()=\"$label\"]");
   
   @@ -95,7 +95,7 @@ public function fieldUIAddExistingField($bundle_path, $existing_storage_name, $l
   +    $this->assertSession()->elementExists('xpath', "//table[@id=\"field-overview\"]//tr/td[1 and text()=\"$label\"]");
   
   @@ -123,7 +123,7 @@ public function fieldUIDeleteField($bundle_path, $field_name, $label, $bundle_la
   +    $this->assertSession()->elementNotExists('xpath', '//table[@id="field-overview"]//span[@class="label-field" and text()="' . $label . '"]');
   

   Same

7. +++ b/core/modules/file/tests/src/Functional/FileFieldDisplayTest.php
   @@ -218,7 +218,7 @@ public function testDescriptionDefaultFileFieldDisplay() {
   +    $this->assertSession()->elementTextContains('xpath', '//a[@href="' . $node->{$field_name}->entity->createFileUrl(FALSE) . '"]', $description);
   
   @@ -228,7 +228,7 @@ public function testDescriptionDefaultFileFieldDisplay() {
   +    $this->assertSession()->elementTextContains('xpath', '//a[@href="' . $node->{$field_name}->entity->createFileUrl(FALSE) . '"]', $description);
   

   The link* methods returning the element would help here, this will have to do otherwise.

8. +++ b/core/modules/file/tests/src/Functional/FileManagedFileElementTest.php
   @@ -24,7 +24,7 @@ class FileManagedFileElementTest extends FileFieldTestBase {
   +    $this->assertSession()->elementExists('xpath', '//input[@name="files[nested_file]" and @size="13"]');
   

   This could be fieldExists() and then a separate attribute check.

9. +++ b/core/modules/shortcut/tests/src/Functional/ShortcutSetsTest.php
   @@ -151,7 +151,7 @@ public function testShortcutSetSwitchNoSetName() {
   -    $this->assertFieldByXPath("//input[@name='label' and contains(concat(' ', normalize-space(@class), ' '), ' error ')]", NULL, 'The new set label field has the error class');
   +    $this->assertSession()->elementExists('xpath', "//input[@name='label' and contains(concat(' ', normalize-space(@class), ' '), ' error ')]");
   

   Can we use fieldExists() and then hasClass() here?

10. +++ b/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php
    @@ -41,13 +41,13 @@ public function testModuleListForm() {
    +    $this->assertSession()->elementExists('xpath', "//a[contains(@href, '/system-test/configure/bar') and text()='Configure ']/span[contains(@class, 'visually-hidden') and text()='the System test module']");
    ...
    +    $this->assertSession()->elementExists('xpath', "//a[contains(@href, '/admin/people/permissions#module-system_test') and @title='Configure permissions']");
    ...
    +    $this->assertSession()->elementExists('xpath', "//a[contains(@href, '/admin/help/system_test') and @title='Help']");
    

    Again a shame that the link* methods don't return the element, as that could mean these could be broken into separate assertions.

11. +++ b/core/modules/system/tests/src/Functional/Module/PrepareUninstallTest.php
    @@ -159,7 +159,7 @@ public function testUninstall() {
         $button_xpath = '//input[@type="submit"][@value="Delete all entity test without label entities"]';
    -    $this->assertNoFieldByXPath($button_xpath, NULL, 'Button with value "Delete all entity test without label entities" not found');
    +    $this->assertSession()->elementNotExists('xpath', $button_xpath);
    

    buttonNotExists()?

12. +++ b/core/modules/user/tests/src/Functional/UserCreateTest.php
    @@ -78,7 +78,7 @@ public function testUserAdd() {
    -    $this->assertFieldByXPath('//input[@type="radio" and @id="edit-status-1" and @checked="checked"]', NULL, 'Default setting for user status is active.');
    +    $this->assertSession()->elementExists('xpath', '//input[@type="radio" and @id="edit-status-1" and @checked="checked"]');
    

    Can we use checkboxChecked()? (even though it's a radio, it seems like it should still work)

13. +++ b/core/modules/user/tests/src/Functional/UserLoginTest.php
    @@ -172,7 +172,7 @@ public function assertFailedLogin($account, $flood_trigger = NULL) {
    -    $this->assertNoFieldByXPath("//input[@name='pass' and @value!='']", NULL, 'Password value attribute is blank.');
    +    $this->assertSession()->elementNotExists('xpath', "//input[@name='pass' and @value!='']");
    

    Surely we actually want fieldValueEquals('pass', '') here?

For #56.6 I agree about the last case but the first two are just td[1] and that will always only be one element anyway.

Ahh yes, sorry, you're absolutely right. The other changes look great so RTBC if bot agrees.

I don't think that's the issue, it's that either the field exists and is empty, or the field doesn't exist at all (in the case of the flood protection being triggered). The old XPath tried to cater for both these, but it's better to assert the two cases separately.

+1 to proper escaping, looks good to me.

Patch no longer applies.

Needs a re-roll.

I'll do it.

Good clean-up, nice to see so many of the assertions get more readable in the process.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!