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
NodeDisplayConfigurableTest relies heavily on RDF to detect specific elements, but we are removing RDF from core.
Steps to reproduce
Proposed resolution
Refactor the test so it no longer relies on RDF.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#9 | 3303278-9.patch | 3.56 KB | smustgrave |
#9 | interdiff-2-9.txt | 955 bytes | smustgrave |
Comments
Comment #2
smustgrave CreditAttribution: smustgrave at Mobomo commentedTook a shot
Comment #3
longwaveThanks for picking this up.
Not sure we should just remove these, we need to find another way of identifying the div.
Should this also check $html_element still?
Comment #4
smustgrave CreditAttribution: smustgrave at Mobomo commented#3.1
Do we need those assertions? The author value is checked below those. Seemed those assertions were testing for the metadata that is now gone.
#3.2
Without rdf seems to wrap in 1 less div or span
Comment #5
longwaveThanks for the swift response.
#4.1 Agree after looking again we don't need these, they aren't testing anything useful except the RDF output which is now gone.
#4.2 The number of elements is the same, the issue is that it's difficult to find the correct one now. The Stark output looks like this:
The RDF output conveniently tagged the author name, but now it's pretty difficult to find except by counting divs - and that will make the test more fragile for future changes IMO, so I think it's fine just to assert the existence of the output.
Two more minor review comments:
The quotes can be removed from around $uid_selector.
Not sure this is necessary; we check elementTextContains() for this above, so we already know that it exists?
Comment #6
longwaveComment #7
smustgrave CreditAttribution: smustgrave at Mobomo commented5.2 added that best when stable and stable9 are run the date doesn’t get an attribute or class so the assertion fails
Will address 5.1
Comment #8
longwaveRe #5.2 but why was this added in the first place? I don't see what this is replacing.
Comment #9
smustgrave CreditAttribution: smustgrave at Mobomo commentedThe original code was
$assert->elementExists('css', 'span[property="schema:dateCreated"]');
Changed it up to look for the css class instead but that only works for themes with classes
Addressed 5.1
Comment #10
longwaveOK, the work here is done, I think this is ready to go in.
Comment #12
catchCommitted/pushed to 10.1.x, cherry-picked to 9.5.x and 9.4.x, thanks!