When running the Behat test suite on a Panopoly child distribution, we sometimes run into issues where a test will pass when run singly but fail when run with the full suite (or vice versa). The most common variables that cause this are Panopoly's "add content preview" and "live preview" settings. These settings only affect Javascript scenarios, but there might be some that also affect non-JS scenarios.
What do you think of adding BeforeScenario and AfterScenario functions to clean this up?
One way to do it would be to store the value of both variables before the test and then restore them afterward. In this case, every test that touches live preview or add content would have to set the correct value itself, even if it used the defaults.
Another possibility would be to store the existing value and then set both variables to the default. Then a test that used the default settings wouldn't have to specify, but a test that didn't use the default settings would. After the scenario the variables would be set back to their stored values.
In either case, this should be non-destructive when run on a site with any existing setup. Unless a test crashed (not just failed, but actually blew up and couldn't complete), everything would be back to the previous state after any single test or suite.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | panopoly_test-behat-defaults-3081254-7.patch | 34.82 KB | cboyden |
Comments
Comment #2
dsnopekWhile I haven't encountered this problem personally, in principal, I think doing this sounds like a great idea!
As far as implementation, we could even have an array on the class defining the @AfterScenario which stores the variables to reset and their values. Then we could have any step that modifies a variable put the original value in that array. That would make the implementation generic and extensible to other variables that could cause problems in the future.
Comment #3
cboyden commentedHere's a patch that captures the state of the variables before the scenario and restores it afterward, and relies on the defaults for live previews (automatic) and add content previews (single) except where the scenario specifically calls for something different.
Comment #4
cboyden commentedThis patch goes a bit further in removing test steps that disable live previews and or add content previews. In order to make the view pane tests work with default settings, I had to add some descriptions to the test views to disambiguate links.
This raises some larger questions though: There are many tests remaining that disable live previews. Why was that done initially? If we don't need live previews on all the widget tests where they are turned off, should we change the approach so that we default to having live previews disabled during the test run, and only turn it on where it's the thing being tested? Does having live preview on for the tests where it's not already turned off provide any advantage? Live preview can be finicky, do we catch any regressions or odd behavior when we leave live previews on for some extra tests?
Comment #5
dsnopekI believe it was done to ensure consistent test results. Live previews can add extra controls and Javascript to the page, which can sometimes effect what Behat does (for example, if we're trying to grab the nth element of a type), and we didn't want the test to pass or fail depending on the preview setting.
That makes a lot of sense to me. If we disable live preview in your new
saveConfig()method by default, then we can remove that line from a ton of test, which would be nice -- one less thing to copy-paste when making new tests.I don't think there's really an advantage. If we find a case where live preview breaks that wasn't tested, we should add a dedicated live preview test for it after it's fixed.
Comment #6
cboyden commentedHere's a new patch that disables live preview for all of the tests except livepreview.feature. Tests are running at https://travis-ci.org/github/cboyden/panopoly/builds/774218703.
Comment #7
cboyden commentedUpdated patch to work around #3218367: Reusable widgets can't be created when live previews are disabled for now, and fix some issues in the FAPE and view modes scenarios. Tests are running at https://travis-ci.org/github/cboyden/panopoly/builds/774228924.
Comment #9
dsnopekThis looks great to me!
I did one more test on Travis CI here:
https://travis-ci.org/github/panopoly/panopoly/jobs/774547436
Committed :-)
Comment #10
dsnopekMarking this as needing to be ported to Panopoly 2.x, so I don't lose track of it
Comment #11
dsnopekHere's an MR for Panopoly 2.x-8.x:
https://gitlab.com/panopoly/panopoly/-/merge_requests/7
This mainly just brings the .feature files in line with their Panopoly 1.x counterparts, the actually functionality isn't used because there are no defaults we need to set in Panopoly 8.x-2.x yet. This also brings over some feature files that were missing on the 2.x-8.x branch, because it was branched before they were created.
Comment #13
dsnopekMerged!