Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
From #3129002: Replace usages of deprecated AssertLegacyTrait::assert():
In cases of constructs like
$view_link = $this->xpath('//div[@class="messages"]//a[contains(@href, :href)]', [':href' => 'aggregator/sources/']);
$this->assert(isset($view_link), 'The message area contains a link to a feed');
we are in fact failing to test appropriately: $this->xpath returns an empty array here, since the xpath selector is //div[@class="messages"] and in fact since the test was written the class changed to 'messages-list'. But an empty array is set as a variable, so the test passes anyway. I corrected by changing the selector to //div[@data-drupal-messages], and using the WebAssert::elementExists method.
Steps to reproduce
Proposed resolution
Review tests that use $this->xpath() and then an assertion using isset to ensure they are testing what they are meant to test.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Comments
Comment #2
mondrakeSwitching to meta since this will likely need multiple issues.
Comment #3
andypostMaybe that's remains/leftovers from css-select conversion?
Comment #4
mondrakeThe POC #3168788: Convert assertions involving use of xpath to WebAssert, aggregator module was committed with the comment
which may not be a blessing to move on with module-specific issues, but at least is an encouragement in that sense :)
Comment #5
mondrakeComment #6
mondrakeComment #7
mondrakeComment #8
mondrakeComment #9
mondrakeComment #10
mondrakeComment #11
mondrakeComment #12
mondrakeComment #13
mondrakeComment #14
mondrakeComment #15
mondrakeComment #16
mondrakeComment #17
paulocsComment #18
mondrakeComment #19
xjmI don't think #3168788: Convert assertions involving use of xpath to WebAssert, aggregator module should have been committed with a per-module scope (and therefore am not comfortable committing all the other issues scoped this way). As per: https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett...
Can we rethink how these are scoped, please, instead of having the overhead of a bunch of little 5K patches? What is the motivation for doing this per module?
Comment #20
xjmI've postponed the child issues pending a discussion and decision around #19. While these can be more difficult than your typical find-and-replace, it's still a lot of overhead to have so many issues in the 2-5K patch range. Are there patters we can establish? Etc.
Comment #21
longwave@mondrake and I have discussed this both in Slack and in #3168788: Convert assertions involving use of xpath to WebAssert, aggregator module. We are open to suggestions for improved scoping but the problem is that this one isn't just a find-and-replace job like many of these PHPUnit improvement metas have been. There doesn't seem to be any obvious way to group them otherwise; each individual case needs careful checking to ensure it's actually testing what it is intended to test, and so smaller patches make this easier to both write and review.
Comment #22
mondrake#21++
I would also say that the approach per module allows participation by many more contributors, and with different levels of experience, since there will be issues with different levels of complexity.
The alternative of a monster patch is practically unmanageable because of size, needs of rerolls, etc.
Comment #23
longwaveOne other option is to go down the route of the unused variable removals and have one issue for each affected line (or perhaps test method) but that seems even more work for core committers than by-module.
Comment #25
longwaveOpened #3184632: Convert assertions involving use of xpath on submit inputs to WebAssert to try scoping these in a different way. I suggest to anyone following this issue to refrain from opening similar issues just yet, in case this approach is not accepted either.
Comment #26
longwaveEncouraged by the acceptance of the previous issue I've identified and opened #3187309: Convert assertions involving use of xpath on select and option elements to WebAssert for the next batch of these.
Comment #27
longwaveOpened #3189607: Convert assertions involving use of xpath on checkboxes to WebAssert
Comment #28
mondrakeClosed all open issues of the now abandoned per-module approach,
Comment #29
mondrakeComment #30
mondrakeComment #31
mondrakeComment #32
mondrakeComment #33
longwaveOpened #3202915: Convert assertions involving use of xpath on textareas to WebAssert
Comment #34
mondrakeFiled #3203476: Convert assertions involving use of xpath on divs to WebAssert
Comment #35
mondrakeComment #37
mondrakeComment #39
mondrake#3253715: Replace assertions involving calls to isset() on xpath results in functional tests with WebAssert calls landed, and I think now we can try to address the remaining cases in a single patch. Do we change this from a meta to a concrete issue, or open a new one child of this?
Comment #40
mondrakeComment #41
mondrakeA few conversions.
Comment #42
mondrakeComment #43
mondrakeComment #45
mondrakeFiled #3265574: Convert remaining assertions involving use of xpath to WebAssert, where possible to keep this meta clear from concrete patches.
Comment #46
mondrake#3265574: Convert remaining assertions involving use of xpath to WebAssert, where possible is done, so I think at this point this meta can be closed.