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

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

HI please review this patch.
Fix failed test cases.

When I do a code base search for $this->assertFieldById( and $this->assertNoFieldById( I still get results. More code changes are needed.

@daffie Thanks

RE #17 has been done can you please review this patch.

Working on failed tests

Need to update test case.

Did a quick review and found major issues on previous patches.

1. Replaced assertSession()->field(Not)Exists to assertSession()->fieldValue(Not)Equals as there is actually verification of field values.
2. Replaced assertSession()->field(Not)Exists to assertSession()->button(Not)Exists where button is verified.
3. Replaced Replaced assertSession()->field(Not)Exists to Replaced assertSession()->checkbox(Not)Checked for checkboxes
4. Removed all assert(No)FieldById in Legacy methods

Fixed PHP lint failure.

Fixed Failing tests.

Patch failed to apply , kindly follow a new patch.

Hi @Hardik_Patel_12,

Could you please add interdiff its drupal ci say the patch is pass and I am also not able to apply the #35 attached file failer report.

Thanks,
Shakti.

Please review the following patch.

Thanks

Hi @hash6,

Just reroll patch from #35

@shaktik after applying #43 when I do a code base search for $this->assertFieldById( and $this->assertNoFieldById( I still get results. More code changes are needed.

File: core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php still get results with $this->assertFieldById( and $this->assertNoFieldById( .

Working on failed test cases on #40.

Please review the patch.
Thanks

Please review this patch.
Thanks,

Hi @mondrake

I have made changes as per #50, kindly review it.

Need a deprecation test for the deprecated methods, and the deprecation silencers removed from the DeprecationListenerTrait.

#50 hasn't been fully addressed.

Removing deprecation silencers removed from the DeprecationListenerTrait.

The failure is in testFieldAssertsForButton from core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php.
Updated testFieldAssertsForButton method from core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php with new assertions.

Thanks @mohrerao!

Need a deprecation test for the deprecated methods, and the deprecation silencers removed from the DeprecationListenerTrait.

1. "Need a deprecation test for the deprecated methods" has not been addressed yet, still. ❌
2. "and the deprecation silencers removed from the DeprecationListenerTrait." ✅
3. Needs reroll

Postpone on #3139406: Replace usages of AssertLegacyTrait::assert(No)FieldByName, that is deprecated, Some usages in BrowserTestBase could be removed due to the replacement is the same, and overlapped.

1. +++ b/core/modules/comment/tests/src/Functional/CommentAdminTest.php
   @@ -231,12 +231,13 @@ public function testEditComment() {
   -    $this->assertFieldById('edit-mail', $anonymous_comment->getAuthorEmail());
   +    $this->assertSession()->fieldExists('edit-mail');
   +    $this->assertSession()->fieldValueEquals('edit-mail', $anonymous_comment->getAuthorEmail());
   

   fieldValueEquals() will throw an exception anyway if the field doesn't exist.

2. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
   @@ -458,7 +458,8 @@ public function testDefaultValue() {
   -    $this->assertFieldById($element_id, '', 'The default value widget was empty.');
   +    $this->assertSession()->fieldExists($element_id);
   +    $this->assertSession()->fieldValueEquals($element_id, '');
   

   Same

3. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
   @@ -474,7 +475,8 @@ public function testDefaultValue() {
   -    $this->assertFieldById($element_id, '1', 'The default value widget was displayed with the correct value.');
   +    $this->assertSession()->fieldExists($element_id);
   +    $this->assertSession()->fieldValueEquals($element_id, '1');
   

   Same

4. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
   @@ -506,7 +508,8 @@ public function testDefaultValue() {
   -    $this->assertFieldById($element_id, '', 'The default value widget was displayed when field is hidden.');
   +    $this->assertSession()->fieldExists($element_id);
   +    $this->assertSession()->fieldValueEquals($element_id, '');
   

   Same

5. +++ b/core/modules/locale/tests/src/Functional/LocaleConfigTranslationTest.php
   @@ -205,7 +205,8 @@ public function testConfigTranslation() {
   +    $this->assertSession()->fieldExists('edit-label');
   +    $this->assertSession()->fieldValueEquals('edit-label', 'Website feedback');
   

   Same

6. +++ b/core/modules/system/tests/src/Functional/Entity/EntityRevisionsTest.php
   @@ -177,8 +177,10 @@ protected function runRevisionsTests($entity_type) {
   +    $this->assertSession()->fieldExists('edit-name-0-value', NULL);
   +    $this->assertSession()->fieldValueEquals('edit-name-0-value', $entity->name->value);
   +    $this->assertSession()->fieldExists('edit-translatable-test-field-0-value', NULL);
   +    $this->assertSession()->fieldValueEquals('edit-translatable-test-field-0-value', $entity->translatable_test_field->value);
   

   Same

7. +++ b/core/modules/taxonomy/tests/src/Functional/Views/TaxonomyIndexTidUiTest.php
   @@ -121,7 +121,7 @@ public function testFilterUI() {
   -    $this->assertFieldById('edit-options-value', NULL);
   +    $this->assertSession()->fieldExists('edit-options-value', NULL);
   

   We don't need the second argument to fieldExists() here.

8. +++ b/core/modules/user/tests/src/Functional/UserCreateTest.php
   @@ -76,8 +76,10 @@ public function testUserAdd() {
   +    $this->assertSession()->fieldExists('edit-status-0');
   +    $this->assertSession()->fieldValueEquals('edit-status-0', '1');
   +    $this->assertSession()->fieldExists('edit-status-1');
   +    $this->assertSession()->fieldValueEquals('edit-status-1', '1');
   

   We don't need to check existence separately.

9. +++ b/core/modules/views/tests/src/Functional/BulkFormTest.php
   @@ -56,12 +56,12 @@ public function testBulkForm() {
   -    $this->assertFieldById('edit-action', NULL, 'The action select field appears.');
   +    $this->assertSession()->fieldExists('edit-action', NULL);
   

   We don't need the second argument.

10. +++ b/core/modules/views/tests/src/Functional/BulkFormTest.php
    @@ -56,12 +56,12 @@ public function testBulkForm() {
    -      $this->assertFieldById('edit-node-bulk-form-' . $i, NULL, new FormattableMarkup('The checkbox on row @row appears.', ['@row' => $i]));
    +      $this->assertSession()->fieldExists('edit-node-bulk-form-' . $i, NULL);
    

    Same

11. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormTest.php
    @@ -115,7 +115,8 @@ public function testExposedIdentifier() {
    +    $this->assertSession()->fieldExists(Html::getId('edit-' . $identifier), NULL);
    +    $this->assertSession()->fieldValueEquals(Html::getId('edit-' . $identifier), 'article');
    

    We don't need to check existence separately.

12. +++ b/core/modules/views/tests/src/Functional/Plugin/FilterTest.php
    @@ -185,9 +185,9 @@ public function testLimitExposedOperators() {
    -    $this->assertFieldById('edit-nid-value');
    ...
    +    $this->assertSession()->fieldExists('edit-nid-value');
    

    The default value of '' passed to assertFieldById() also checks the value is an empty string, not just existence.

13. +++ b/core/modules/views/tests/src/Functional/Plugin/FilterTest.php
    @@ -203,9 +203,9 @@ public function testLimitExposedOperators() {
    -    $this->assertFieldById('edit-nid-value');
    -    $this->assertFieldById('edit-nid-min');
    -    $this->assertFieldById('edit-nid-max');
    +    $this->assertSession()->fieldExists('edit-nid-value');
    +    $this->assertSession()->fieldExists('edit-nid-min');
    +    $this->assertSession()->fieldExists('edit-nid-max');
    

    Same

14. +++ b/core/modules/views_ui/tests/src/Functional/ExposedFormUITest.php
    @@ -73,10 +73,12 @@ public function testExposedAdminUi() {
    +    $this->assertSession()->fieldExists('edit-options-operator-in');
    +    $this->assertSession()->fieldValueEquals('edit-options-operator-in', 'in');
    +    $this->assertSession()->fieldExists('edit-options-operator-not-in');
    +    $this->assertSession()->fieldValueEquals('edit-options-operator-not-in', 'in');
    

    We don't need to check for existence separately.

15. +++ b/core/modules/views_ui/tests/src/Functional/ExposedFormUITest.php
    @@ -84,10 +86,12 @@ public function testExposedAdminUi() {
    +    $this->assertSession()->fieldExists('edit-options-operator-in');
    +    $this->assertSession()->fieldValueEquals('edit-options-operator-in', 'in');
    +    $this->assertSession()->fieldExists('edit-options-operator-not-in');
    +    $this->assertSession()->fieldValueEquals('edit-options-operator-not-in', 'in');
    

    Same

16. +++ b/core/modules/views_ui/tests/src/Functional/ExposedFormUITest.php
    @@ -103,24 +107,26 @@ public function testExposedAdminUi() {
    -    $this->assertNoFieldById('edit-options-expose-label', '', 'Make sure no label field is shown');
    +    $this->assertSession()->fieldNotExists('edit-options-expose-label', NULL);
    

    The second argument is unnecessary.

17. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -425,14 +438,13 @@ public function testFieldAssertsForTextfields() {
    -    $this->assertNoFieldById('name');
    -    $this->assertNoFieldById('name', 'not the value');
    -    $this->assertNoFieldById('notexisting');
    -    $this->assertNoFieldById('notexisting', NULL);
    +    $this->assertSession()->fieldNotExists('name[0][value]', NULL);
    +    $this->assertSession()->fieldValueNotEquals('name', 'not the value');
    +    $this->assertSession()->fieldNotExists('notexisting');
    

    Where did 'name[0][value]' come from?

18. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -441,7 +453,7 @@ public function testFieldAssertsForTextfields() {
    -      $this->assertNoFieldById('edit-name', NULL);
    +      $this->assertSession()->fieldValueEquals('name', '');
    

    Shouldn't this be fieldNotExists()?

19. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -449,15 +461,14 @@ public function testFieldAssertsForTextfields() {
    +    $this->assertSession()->fieldExists('edit-name', NULL);
    +    $this->assertSession()->fieldValueEquals('edit-name', 'Test name');
    +    $this->assertSession()->fieldExists('edit-description', NULL);
    +    $this->assertSession()->fieldValueEquals('edit-description', '');
    

    We don't need a second argument to fieldExists()

20. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -590,32 +600,23 @@ public function testFieldAssertsForOptions() {
    +    // Verify if the test passes with button ID.
    +    $this->assertSession()->buttonExists('edit-save', NULL);
    +    // Verify if the test passes with button Value.
    +    $this->assertSession()->buttonExists('Save', NULL);
    +    // Verify if the test passes with button Name.
    +    $this->assertSession()->buttonExists('op', NULL);
    

    We don't need a second argument to buttonExists()

21. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -590,32 +600,23 @@ public function testFieldAssertsForOptions() {
    +    // Verify if the test passes with button ID.
    +    $this->assertSession()->buttonNotExists('i-dont-exist', NULL);
    +    // Verify if the test passes with button Value.
    +    $this->assertSession()->buttonNotExists('I dont exist', NULL);
    +    // Verify if the test passes with button Name.
    +    $this->assertSession()->buttonNotExists('no', NULL);
    

    Same

22. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -590,32 +600,23 @@ public function testFieldAssertsForOptions() {
    -    $this->assertFieldByName('duplicate_button', 'Duplicate button 1');
    -    $this->assertFieldByName('duplicate_button', 'Duplicate button 2');
    -    $this->assertNoFieldByName('duplicate_button', 'Rabbit');
    +    $this->assertSession()->buttonExists('Duplicate button 1');
    +    $this->assertSession()->buttonExists('Duplicate button 2');
    

    Missing the third assertion here

23. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -678,27 +679,27 @@ public function testFieldAssertsForCheckbox() {
    -    $this->assertFieldById('edit-checkbox-enabled', NULL);
    -    $this->assertFieldById('edit-checkbox-disabled', NULL);
    +    $this->assertSession()->fieldExists('edit-checkbox-enabled', NULL);
    +    $this->assertSession()->fieldExists('edit-checkbox-disabled', NULL);
    

    We don't need the second argument again

24. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -678,27 +679,27 @@ public function testFieldAssertsForCheckbox() {
    -      $this->assertNoFieldById('edit-checkbox-disabled', NULL);
    +      $this->assertSession()->fieldNotExists('edit-checkbox-disabled', NULL);
    

    Same

Working on addressing comment #61.

Here I have tried to address points 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23 and 24 of comment #61.

I have addressed 1 to 6 points suggested in comment #61.
Please review

Rerolled the patch in #66

Hello,
Patch #70 looks good to me. I didn't find no AssertLegacyTrait::assert(No)FieldById any more.
Moving to RTBC.

Cheers, Paulo.

Looks good but needs a re-roll.

Patch re-rolled.

One more issue:

+    // Verify if the test passes with button ID.
+    $this->assertSession()->buttonNotExists('i-dont-exist');
+    // Verify if the test passes with button Value.
+    $this->assertSession()->buttonNotExists('I dont exist');
+    // Verify if the test passes with button Name.
+    $this->assertSession()->buttonNotExists('no');

this is failing cspell due to 'dont', probably need to change to i-do-not-exist and I do not exist or I don't exist.

Updated the patch in #73 with the minor corrections mentioned in #75... Kindly review.

Thanks!

Committed 71813a8 and pushed to 9.1.x. Thanks!