Skip to content

Add visual diffing proof-of-concept.#7382

Closed
cjcenizal wants to merge 5 commits intoelastic:masterfrom
cjcenizal:poc/visual-diffing
Closed

Add visual diffing proof-of-concept.#7382
cjcenizal wants to merge 5 commits intoelastic:masterfrom
cjcenizal:poc/visual-diffing

Conversation

@cjcenizal
Copy link
Copy Markdown
Contributor

@cjcenizal cjcenizal commented Jun 7, 2016 •
edited
Loading

Changes

  • Add scripts/compareScreenshots.js.
  • Add gm dependency.
  • Temporarily introduce error to app-link.
  • Rename test/output to test/screenshots.

Goal

During CSS refactors, we can use this tool to make sure we haven't introduced any CSS regressions.

How to test

  • Install the graphicsmagick and imagemagick command-line tools (on OS X this can be done with Homebrew: brew install imagemagick and brew install graphicsmagick).
  • Install the gm devDependency with npm i.
  • Run the functional tests, which will generate git-ignored screenshots in test/screenshots/session.
  • Run node scripts/compareScreenshots, which will traverse the recently captured screenshots, compare them against those committed in test/screenshots/baseline and output image diffs to test/screenshots/diff.

Then you can look at the generated diff images to see if anything has dramatically changed (i.e. broken). In this proof-of-concept, you'll see the change introduced in app_switcher.less surfaced in the diffs.

Looking ahead

The next steps with this tool can be:

  • Test this tool and make sure it works on Windows.
  • Package it with a better interface (and perhaps extract it into its own repo?).
  • Take more screenshots during the functional tests so we can capture a broader range of our UI state.
  • Start using it during the CSS refactor and see how useful it is (CSS cleanup #7364).

@cjcenizal
Add visual diffing proof-of-concept.
fe817a8 
- Add scripts/compareScreenshots.js.
- Add gm dependency.
- Temporarily introduce error to app-link.
- Rename test/output to test/screenshots.
@cjcenizal cjcenizal force-pushed the poc/visual-diffing branch from fa30211 to fe817a8 Compare June 7, 2016 00:12
@LeeDr
Copy link
Copy Markdown

LeeDr commented Jun 7, 2016

FYI, I tried installing gm, imagemagick, and graphicsmagick all with npm on Windows in a gitbash shell. gm and imagemagick appear to have worked.

Lee@LeeD-ASUS MINGW64 /c/git/kibana (elasticDumpDiscover)
$ npm install gm
gm@1.22.0 node_modules\gm
├── array-parallel@0.1.3
├── array-series@0.1.5
└── debug@2.2.0 (ms@0.7.1)

Lee@LeeD-ASUS MINGW64 /c/git/kibana (elasticDumpDiscover)
$ npm install imagemagick
imagemagick@0.1.3 node_modules\imagemagick

Lee@LeeD-ASUS MINGW64 /c/git/kibana (elasticDumpDiscover)
$ npm install graphicsmagick

> graphicsmagick@0.0.1 preinstall C:\git\kibana\node_modules\graphicsmagick
> node-waf clean || (exit 0); node-waf configure build

node-waf was unexpected at this time.
npm ERR! Windows_NT 10.0.10586
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install" "graphicsmagick"
npm ERR! node v4.3.2
npm ERR! npm  v2.14.12
npm ERR! code ELIFECYCLE

npm ERR! graphicsmagick@0.0.1 preinstall: `node-waf clean || (exit 0); node-waf configure build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the graphicsmagick@0.0.1 preinstall script 'node-waf clean || (exit 0); node-waf configure build'.
npm ERR! This is most likely a problem with the graphicsmagick package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node-waf clean || (exit 0); node-waf configure build
npm ERR! You can get their info via:
npm ERR!     npm owner ls graphicsmagick
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     C:\git\kibana\npm-debug.log

So I installed grapicsmagick Q8 64 bit for Windows from here;
https://sourceforge.net/projects/graphicsmagick/files/graphicsmagick-binaries/1.3.24/GraphicsMagick-1.3.24-Q8-win64-dll.exe/download

I thought the session screenshots would be saved in my kibana/test/session but they're going to C:\screenshots\session

When I try to run node scripts/compareScreenshots I get this;

Lee@LeeD-ASUS MINGW64 /c/git/kibana (cjcenizal-poc/visual-diffing)
$ node scripts/compareScreenshots
C:\git\kibana\scripts\compareScreenshots.js:10
    const screenshots = files.filter(file => file.indexOf('.png') !== -1);
                             ^

TypeError: Cannot read property 'filter' of undefined
    at C:\git\kibana\scripts\compareScreenshots.js:10:30
    at FSReqWrap.oncomplete (fs.js:82:15)

And when I open compareScreenshots in my editor is shows "handle" is not defined at line 19

@kimjoar kimjoar mentioned this pull request Jun 7, 2016
Closed
@cjcenizal
Copy link
Copy Markdown
Contributor Author

cjcenizal commented Jun 7, 2016

OK, I've tried getting this to work on my Windows VM but developing in the VM is too painful and slow. After discussion with @ycombinator, @panda01, and @LeeDr, I'm going to close this and:

  • Create a separate repo to house this tool, documenting use cases and providing a caveat that it's only been tested with OS X.
  • Create a new PR that adds the repo as a dependency, and surfaces it as an npm script.

Pinging @bevacqua to keep him looped in!

@cjcenizal cjcenizal closed this Jun 7, 2016
@cjcenizal cjcenizal deleted the poc/visual-diffing branch June 7, 2016 20:51
@bevacqua
Copy link
Copy Markdown
Contributor

bevacqua commented Jun 7, 2016

Have you looked at options to do stuff like this? There's several. Some:

@cjcenizal
Copy link
Copy Markdown
Contributor Author

cjcenizal commented Jun 7, 2016

@bevacqua Nice! Thanks! I think Pageres is too closely coupled to the screenshot-capture process but image-diff looks like it might be perfect for our use case. I'll take a closer look before moving forward with a custom tool.

@Bargs
Copy link
Copy Markdown
Contributor

Bargs commented Jun 7, 2016

There's also https://github.com/Huddle/Resemble.js, which I thought @LeeDr played around with before?

@cjcenizal
Copy link
Copy Markdown
Contributor Author

cjcenizal commented Jun 8, 2016 •
edited
Loading

@Bargs Resemble is a bust, since its node port has fell out of regular mainteance. 😞

@bevacqua We have a winner with image-diff, though! This library is exactly what I would have liked to write. It has a fundamentally identical implementation to what I've done in this branch, the output is exactly the same, and it already presents the interface I would like to use. Moving forward with this one. Great suggestion!

@cjcenizal cjcenizal mentioned this pull request Jun 8, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cjcenizal @LeeDr @bevacqua @Bargs