Problem/Motivation

In #3167880: [meta] Convert assertions involving use of xpath to WebAssert, where possible we identified that use of $this->xpath() can be dangerous as we aren't always testing what we expect to test. In WebAssert we have more appropriate methods to find and assert HTML elements, so we should use them where possible.

This issue is scoped to find XPath selectors for <input type="checkbox"> elements and convert those where possible.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3189607-8.patch9.76 KBanmolgoyal74
#6 interdiff_2_6.txt914 bytesspokje
#6 3189607-6.patch9.76 KBspokje
#4 3189607-4.patch9.82 KBPooja Ganjage
#2 3189607-2.patch9.82 KBlongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3189607-2.patch9.82 KB

Initial pass at this, I think I missed some instances as the XPath doesn't always explicitly specify checkboxes.

Status: Needs review » Needs work

The last submitted patch, 2: 3189607-2.patch, failed testing. View results

Pooja Ganjage’s picture

Pooja Ganjage commented
StatusFileSize
new 3189607-4.patch9.82 KB

Hi,

Tried to solving an issue that raised in #2 comment.

Please review the patch.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage commented
Status: Needs work » Needs review
spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new 3189607-6.patch9.76 KB
new interdiff_2_6.txt914 bytes

@Pooja Ganjage: Please upload an interdiff with each patch. It makes it much easier to review or (in this case) see what went wrong.

Attached patch hopefully fixes the Test Failures in #2

mondrake’s picture

mondrake
Location 🇮🇹
commented
Parent issue: » #3167880: [meta] Convert assertions involving use of xpath to WebAssert, where possible
anmolgoyal74’s picture

anmolgoyal74 commented
StatusFileSize
new 3189607-8.patch9.76 KB

Patch in #6 doesn't apply for file core/modules/system/tests/src/Functional/Module/VersionTest.php
Re-rolled.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Looks good, nice cleanup.

  • catch committed 172e4b5 on 9.2.x 
    Issue #3189607 by Spokje, longwave, Pooja Ganjage, anmolgoyal74: Convert...

  • catch committed eaaf70c on 9.1.x 
    Issue #3189607 by Spokje, longwave, Pooja Ganjage, anmolgoyal74: Convert...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

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

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.