garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.66k stars 602 forks source link

Feat/1432 global scenario settings #1539

Closed dgrebb closed 5 months ago

dgrebb commented 5 months ago

Problem

As described in #1432 and other issues, setting/getting scenario properties at the global level would save time, typing, and testing overhead.

Solution

Spread (...) and logical OR (||) operator.

[!IMPORTANT] This feature is opt-in only, and documented as such

Because configuration at the scenario level will always override any globals set in backstop.json, a user will need to remove the corresponding property in a scenario object.

This is a 100% non-breaking change, thanks to spread...

Supported Global Properties

Added to both Playwright and Puppeteer.

Closes

Notes

An example:

Expand `backstop.json` ```json { "id": "backstop_playwright", "viewports": [ { "label": "phone", "width": 320, "height": 480 }, { "label": "tablet", "width": 1024, "height": 768 } ], "onBeforeScript": "playwright/onBefore.js", "onReadyScript": "playwright/onReady.js", "globals": { "cookiePath": "backstop_data/engine_scripts/cookies.json", "url": "https://garris.github.io/BackstopJS/", "readySelector": "", "delay": 0, "hideSelectors": [".getItBlock"], "removeSelectors": [".logoBlock"], "hoverSelector": "", "clickSelector": "", "postInteractionWait": 1000, "selectors": [], "selectorExpansion": true, "misMatchThreshold" : 0.1, "requireSameDimensions": true }, "scenarios": [ { "label": "BackstopJS Homepage", "cookiePath": "backstop_data/engine_scripts/cookies.json", "url": "https://garris.github.io/BackstopJS/", "referenceUrl": "", "readyEvent": "", "readySelector": "", "delay": 0, "hoverSelector": "", "clickSelector": "", "selectors": [], "selectorExpansion": true, "misMatchThreshold" : 0.1, "requireSameDimensions": true } ], "paths": { "bitmaps_reference": "backstop_data/bitmaps_reference", "bitmaps_test": "backstop_data/bitmaps_test", "engine_scripts": "backstop_data/engine_scripts", "html_report": "backstop_data/html_report", "ci_report": "backstop_data/ci_report" }, "report": ["browser"], "engine": "playwright", "engineOptions": { "args": ["--no-sandbox"] }, "asyncCaptureLimit": 5, "asyncCompareLimit": 50, "debug": false, "debugWindow": false, "archiveReport": true, "scenarioLogsInReports": true } ```

image

Next Steps

We could discuss moving the global onBefore and onReady scripts into the new globals object, but it would be a breaking change, or duplicative (to classify as non-breaking), as we'd have them repeated as such:

  "onBeforeScript": "puppet/onBefore.js",
  "onReadyScript": "puppet/onReady.js",
  "globals": {
    "cookiePath": "backstop_data/engine_scripts/cookies.json",
    "url": "https://garris.github.io/BackstopJS/",
    "onBeforeScript": "puppet/onBefore.js",
    "onReadyScript": "puppet/onReady.js",
JPustkuchen commented 5 months ago

Great work @dgrebb. 🥳
What do you think about renaming "globals" to "defaults" (my fav) or "scenarios-defaults"? I think 'globals' might not be perfect wording? Any better ideas?

dgrebb commented 5 months ago

Thanks @JPustkuchen!

For maintainability, especially because the project has (and will have more!) numerous contributors, I would suggest avoiding the word "default" entirely.

There are some default patterns baked in to the core browser runners which use this naming convention:

These are used when running backstop init, and provide values in the generated, default backstop.json.

To avoid breaking changes, this new "globals" object (or whatever we call it) may not be included in the backstop.json one gets out of the box.

I'm not in love with the name, either. We have a tough job. Perhaps simply using a top level "scenario" (singular) would save us the torment of going down a naming rabbit hole, but I digress.

Will think on this, and also give time for others to discover the party we have going on in here 🎉

Thank you for the suggestions!

dgrebb commented 5 months ago

@garris as discussed I've renamed the global scenario configuration object scenarioDefaults. This is ready for a merge.

@JPustkuchen FYA — you almost got what you asked for :)

JPustkuchen commented 5 months ago

GREAT WORK, thank you @dgrebb !! 🎉

garris commented 5 months ago

Thank you @dgrebb! ⭐️