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.
Follow up to #2863267: Convert web tests of views
Convert \Drupal\views\Tests\ViewElementTest and \Drupal\views\Tests\Plugin\StyleGridTest to kernel tests. Lets see if we can tackle both in the same issue.
Comment | File | Size | Author |
---|---|---|---|
#3 | 2876210-3.patch | 12.31 KB | Lendude |
Comments
Comment #2
LendudeComment #3
Lendude\Drupal\views\Tests\Plugin\StyleGridTest contained one drupalGet to check if a css file was loaded. Looked at the original issue for that #2529748: Regression: Views module CSS is not attached and there is no need for that to be used on the grid style, any style will do for that check, so moved it to the summary test. Feels like a waste to set up a whole new browser test to just check one css file when we can get the same coverage in an existing test.
\Drupal\views\Tests\ViewElementTest was a wild mix of drupalGet and setRawContent. Split it down the middle. In the kernel test the HTML output is slightly different then in the browsertest so needed to modify some xpath queries to get the right result.
Patch is a little hard to read I think (with file move/copies/splits going on at the same time), but this is the most readable version I could roll I think.
Comment #4
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedI agree, it's fine to move it to this test.
Was one to really pay attention while reviewing :) direct patch review was kind-of difficult. Did it the old-fasioned side-by-side review way :)
Review comments:
Seems we lost this check in the conversion?
Seems to be lost as well?
Much better like this :)
Comment #5
Lendude#4.1 and #4.2, it's not so much that we lost the test, it's that we lost the wrapper in the kernel test, so something that isn't loaded in the kernel test version seems to add that wrapper.
The test is still there in the new
\Drupal\Tests\views\Functional\ViewElementTest::testViewElement
so it has coverage.Comment #6
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedI see and agree. As we didn't lose any test coverage lets go forward on this.
Comment #7
alexpottCommitted and pushed 96c7b31 to 8.4.x and 5fca9cc to 8.3.x. Thanks!
Comment #11
alexpottThis can't go into 8.3.x cause #2863267: Convert web tests of views is not in 8.3.x