Problem/Motivation
See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase) and #2809161: Convert Javascript/AJAX testing to use JavascriptTestBase
Proposed resolution
Convert everything to BrowserTestBase. The javascript bit is just using the Field UI javascript, it's not testing anything related to comment module javascript, so lets just keep this simple.
Convert testTwoPagers to JavaScriptTestBase- Convert to BrowserTestBase
Postponed
Needs CommentTestBase for BrowserTestBase from #2866513: Convert web tests to browser tests for comment module
Remaining tasks
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 2809467-16.patch | 2.76 KB | lendude |
| #7 | 2809467-7.patch | 20.39 KB | michielnugter |
| #7 | interdiff-5-7.txt | 1.34 KB | michielnugter |
| #5 | 2809467-5.patch | 20.36 KB | michielnugter |
| #5 | interdiff-3-5.txt | 11.3 KB | michielnugter |
Comments
Comment #2
michielnugter commentedComment #3
michielnugter commentedWork in progress patch. I converted the test and the commentTestBase.
Fails on a JavaScript error, not sure yet whether it's an actual bug or an error in the test.
Comment #4
michielnugter commentedComment #5
michielnugter commentedI updated the test and it now passes. As far as I can see it's a complete conversion and therefore ready for a full review.
I partially converted the CommentTestBase to a functional javascript variant, only the required parts for this test were converted as they were the only parts that could be validated.
A direct convert lead to some problems though as the comment/reply/entity_type/field display's the form for both fields. I had to add a container selector to target the correct fields.
The test is not very heavy on the JavaScript testing, 'only' the form settings part is tested.
Comment #6
klausiPostponing until we figure out the random test fails:
#2830485: \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest fails randomly
#2832672: [PP-1] Upgrade jcalderonzumba/* for better test performance
Comment #7
michielnugter commentedSmall update to use the new api where possible.
The mentioned random fails have been fixed, this issue is good for a proper review then?
Comment #9
michielnugter commentedComment #10
michielnugter commentedSelf-review:
- Base class method 'setCommentSubject' is not used. Either add coverage or remove it.
Comment #11
michielnugter commentedComment #15
lendudeNo longer postponed I'm thinking.
Comment #16
lendudeMy proposed resolution: Convert everything to BrowserTestBase. The javascript bit is just using the Field UI javascript, it's not testing anything related to comment module javascript, so lets just keep this simple and use drupalPostForm.
Comment #17
borisson_This makes a ton of sense, I agree that this doesn't need to be a jstest, great work @Lendude!
Comment #18
alexpottCommitted and pushed 2650c43409 to 8.7.x and 2956937e08 to 8.6.x. Thanks!