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.
Current \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest only test against the Bartik theme. It should test against Stark also like OffCanvasTest
Steps
- Create OutsideInJavascriptTestBase::getThemesToTest which will return list of theme names, bartik and stark for now.
- Update OffCanvasTest to call this instead of having its own list
- Remove
$this->enableTheme('bartik');
form \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest::setUp - Update OutsideInBlockFormTest test functions to use OutsideInJavascriptTestBase::getThemesToTest
Comment | File | Size | Author |
---|---|---|---|
#25 | 2784881-25.patch | 21.17 KB | tedbow |
#25 | interdiff-2784881-23-25.txt | 2.11 KB | tedbow |
#23 | 2784881-23.patch | 20.86 KB | bnjmnm |
#13 | 2784881-13.patch | 20.34 KB | tedbow |
#13 | interdiff-12-13.txt | 565 bytes | tedbow |
Comments
Comment #2
xjmComment #3
tedbowComment #5
tedbowOk. Updated both test class and all test method to use new \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInJavascriptTestBase::getTestThemes()
Now testing bartik and stark everywhere.
Had to change a couple other things
Comment #7
tedbowHad to change some selectors to work with stark.
Also using a random string for the block id to not conflict when the theme changes.
Comment #8
tedbowComment #10
tedbowOk re-rolled
Also added stable and classy to themes that are tested.
Comment #12
tedbowDeleting the block added for each theme at the end of the loop fixes the problem locally.
Comment #13
tedbowI forgot that Seven theme removes contextual links. @see seven_preprocess_block()
So Settings Tray Edit Mode won't work in Seven. But the Off-canvas dialog should still work. In fact Webform is already using the dialog on the admin side.
So added Seven theme to OffCanvasTest.
Comment #15
tedbowComment #17
tedbowWas DrupalCI error. Tests passing now
Comment #18
bnjmnmI'm going to re-roll the patch due to changes made by https://www.drupal.org/node/2862625 and provide a review. Will have in the next few days.
Comment #19
bnjmnmActually looks like re-roll needs to take into account multiple issue commits #2862625 #2786193, #2870146, #2786193 .
I'll have it ready shortly.
Comment #20
bnjmnmUploading a pretty elaborate reroll due to several commits worth of activity since the last patch.
To help with the review process: Based on gitk, these are the issues that altered the FunctionalJavascript test files since the last patch and this reroll.
https://www.drupal.org/node/2862625
https://www.drupal.org/node/2786193
https://www.drupal.org/node/2870146
https://www.drupal.org/node/2784567
Comment #21
bnjmnmComment #22
tedbow@bnjmnm thank for the re-roll!
Yep, it can get tricky with some many other issue affecting the same file.
Just a few things with the re-roll.
I think this comment is new. I am not sure what it is here for.
Is it documenting $element_selector, $label_selector? This would be different every time the test method is invoked.
Change "Offcanvas" to "Off-canvas dialog"
This is line if reverting the "Offcanvas" to "off-canvas" change in the other issue.
Change "Offcanvas" to "off-canvas dialog"
I think these lines were removed but not added back in.
This test is necessary for #2784567: List "Quick edit" before "Configure" in contextual links while in Edit mode
This change is not needed for the re-roll or this issue.
Just a note: I find that finding this non-related issue is easier with a complex re-roll like this when you view the git diff both in regular mode and ignore whitespace mode.
git diff -w
from command line, most GUI's also support this. Very help in cases like this where many lines are the same expect for the fact that they are now indented because they are inside a loop.Also not sure if you know this, or maybe others who are reading this, but when you link to other issues you can use this format
"\[#2784567\]"
(without "\"s I couldn't figure who to show this with out filter changing it.
instead for the whole link then you will get the title and color by status
See
https://www.drupal.org/node/2784567
vs
#2784567: List "Quick edit" before "Configure" in contextual links while in Edit mode
Info on this is down on the bottom here: https://www.drupal.org/filter/tips
Comment #23
bnjmnmUploaded the revised reroll . The comment formatting info is much appreciated.
Comment #24
tedbow@bnjmnm thanks for the updated patch!
Just setting to "Needs Review" so the tests run
Comment #25
tedbow@bnjmnm great. Think the reroll is done.
Just doing some small code cleanup things now.
Comment #26
rajeevkChecked interdiff & then reviewed all code changes after applying patch in #25. All looks good. Marking it RTBC.
Comment #27
webchickVery nice clean-up!
Committed and pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!
Comment #30
tedbow@webchick thanks for committing!
Also @bnjmnm thanks for the work on this issue!
Comment #31
droplet CreditAttribution: droplet commentedIt will be useful if there's a simple way to add my custom theme to the test list also.
Comment #32
tedbow@droplet, all someone would have to do is extend OutsideInBlockFormTest and override the getTestThemes().
Though I do think in general this type of functionality would be useful. It would probably be useful for any functional or javascript functional tests.
They all could be affected by markup or javascript changes implemented by a theme or really a module for that matter.
I am not sure if DrupalCI supports contrib themes having JavscriptFunctional tests but if it did they could add their own test that extends OutsideInBlockFormTest.
Comment #34
tedbowChanging to new settings_tray.module component. @drpal thanks for script help! :)