Rework of many parts for speed and CI reporting #259

onigoetz · 2016-08-18T16:36:58Z

Hello,

first, I'm really sorry for the size of this pull request I know it will be difficult to code review it. We made this changes in a single sprint at our company and couldn't afford to keep everything to separate branches because we needed to push it to our central Jenkins.

Highlights

Completely removed gulp, rely only on fs to move files around
Added a modal to show the images in big, with a slider to compare images in place.
The report doesn't need a server to be seen
To see the report, the files are copied in the project (previously the images were copied into Backstop)
Backstop relies on process.cwd() to generate its path, not ./node_modules/backstopjs
Add a --filter option to filter scenarii by label in test and reference

Variants

You can add "variants" to a scenario, they have the same options as a scenario, but are checked against the main scenario
no reference is created

Browser reporting

Removed resemblejs, use images generated by the CLI reporter. (we have more than 500 tests, running them in the browser just breaks the browser)
Copy the reporter files to the project.
The reporter required to disable the CSP rules of our Jenkins server, not longer the case
Added a way to toggle the summary of scenarii
Added a modal on click on reference or test image with a comparison slider
Tweaked the styles a bit
- Removed custom "circles" to show the number of passed and failed tests : replaced with Bootstrap's "label" because with more than 100 tests, the circle is broken
- Made a more compact "Report" section : removed analysisTime and dimensionDifference (if isSameDimensions is true)
- Changed the font stack (there was a typo)

CLI Reporting

Reporting is always run in the CLI and results added to an object
You can easily add multiple new report types with this new object
junit report was moved to a separate code block to not clutter the main one

Internal changes

paths are made from the current working directory, not backstopjs.
removed the need for an internal hash of the configuration.
the list of comparisons to create is stored in a special temp file.
commands no longer have a before and after this makes the code very complicated to understand and can be replaced by promises inside the main execute
removed "start" and "stop" commands as the server is not needed anymore.

Bugfixes / Regressions

If the tests failed, the return code was still 0
You could not use a JavaScript configuration anymore
Errors in Promises were swallowed

Code Style

Replaced jshint/jscs with eslint
Linted with "semistandard"

Tests

Run tests on travis on more node versions (0.12, 4, 5, 6)

… useful

…ages to backstop. - Do not use resemble-js in the angular part, use images directly - Store configuration hash separately - move compare to a util - create a Report utility to abstract the report writing. - openReport doesn't copy files anymore - remove unneeded utils

…ackstopJS into fix_js_config_loading * 'fix_js_config_loading' of https://github.com/onigoetz/BackstopJS: Apply new configurations

Handle return code correctly

…placed "circle" with Bootstrap badges (circles didn't work with more than 100 tests)

…n a separate step

…e scenarii with "--filter"

machowski · 2016-08-19T17:40:50Z

capture/genBitmaps.js

+  if (lastIndex !== -1 && lastIndex === position) {
+    __dirname = arg.replace(fs.separator + 'genBitmaps.js', '');
+  }
+});


this looks like duplicated code from 'echoFiles.js', maybe extract it to separate helper file?

onigoetz · 2016-08-19T21:39:47Z

I'm not sure that closing the pull request was you goal, or was it ? :)

garris · 2016-08-19T22:00:38Z

see: https://github.com/garris/BackstopJS/tree/version_2_0_onigotez

onigoetz · 2016-08-19T22:13:34Z

Oh great, I didn't see that one.
But how will I be able to push more changes to it ? I have still some changes to make before this PR is completely ready for merge.

And what is your intention with the three branches version_2_0, version_2_0_onigoetz and version_2_0_pradet ?

My work being based on what Julien Pradet made in #241 I don't think this should be reviewed separately.

garris · 2016-08-19T22:28:22Z

Ok. It's merged now.

This is an epic PR! This is an overwhelmingly positive direction for BackstopJS. Thank you for the contribution!

Thanks for clarifying re: #241 -- Sorry I was having trouble deciding how to manage the PR. I guess I will leave 2_0 as is and wait till you are ready to have me pull the PR. In the meantime I am also tracking your public changes so I can test them locally. Sound good?

I am intrigued by scenario.variants.

Is it your intention to abandon change detection of the config?

I am already thinking about the various online resources and tutorials that need to be rewritten with this new, much cleaner interface.

…al reference generating with possibility to generate references for specified set of tests.

Version 2 0 onigotez

onigoetz · 2016-08-29T12:31:47Z

I made a quick check to see if this PR fixes any issues that are already referenced here is the result:

refactor for backstop-core implementation #164 : Added CLI
Clean up CLI output #165 : Done, one part by removing gulp, the other part by cleaning some messages
Check if selected port is free #176 : No server anymore
Old diffs in report #205 : this should be fixed
Failed at the backstopjs@1.3.5 test script 'gulp test'. #219 : As it is a CLI tool, it's not mandatory to run inside NPM run
Backstopjs can't find the relative path config in backstop.json #234 : Using CWD now
Is it possible to use js-based config and use gulp? #236 : Removed Gulp, so not needed anymore
Is it possible to not automatically start the server? #237 : Removed Server, so not needed anymore
npm run test #240 : As it is a CLI tool, it's not mandatory to run inside NPM run
Change location of genConfig #243 : using CWD, so it's easy to generate a configuration anywhere
Generate screenshots for just one scenario #255 : Added filter for reference creation
404s in the report #256 : The pre-made report will prevent crashes
Choose default browser to open report #269 : Done now
npm run test launching windows virtual machine when on Mac #273 : no server anymore

Some PR's as well

Separate creating browser report data from starting server and launching browser. #162 : Server removed
Generate Junit reports on error #199 : Extracted the junit report generation
Use of npm explore to run backstopjs commands #230 : Using a CLI now, probably not needed anymore
Enhancement/open report #261 : Failures no longer prevent report opening

And bonus ones

Add possibility to set filename. #137 : Can be closed as Add possibility to set filename. #247 was merged
Replace some references and leave the others #257 : Can be closed as duplicated

onigoetz · 2016-08-29T12:41:13Z

By the way, the other people to credit for this work are : @borys-rudenko, @Ksushik and @dmitriyilchgmailcom

onigoetz · 2016-08-29T16:34:00Z

Sooo, it seems we're finally there :D

We made all the changes we wanted and feel confident this pull request is robust and good to merge :D

Here is a small screenshot of the comparison modal:

garris · 2016-08-29T23:00:59Z

Super. I will check this out tonight. Thank you!

garris · 2016-08-30T13:45:20Z

Awesome -- thank you! I will be adding some readme info about the 2.0 version soon.

Stéphane Goetz and others added 17 commits August 13, 2016 10:19

Remove jshint/jscs, use eslint, lint all code

16ea895

Remove gulp related stuff, use CWD for paths, makes backtop command…

898a1a0

… useful

Fix js config loading

ec34245

Use require for both JS and JSON config files

fe3e9cc

Update readme

87ca111

Handle promise errors

3f76531

Fix path to comparison

fef964a

Apply new configurations

3cb3d36

Merge branch 'fix_js_config_loading' of https://github.com/onigoetz/B…

936aa65

…ackstopJS into fix_js_config_loading * 'fix_js_config_loading' of https://github.com/onigoetz/BackstopJS: Apply new configurations

Remove before/after in the orchestrator.

9609a69

Handle return code correctly

Add an option to not open the report if we don't want to

3176296

Improved the look of the report, more compact, removed unused CSS, re…

6f426e8

…placed "circle" with Bootstrap badges (circles didn't work with more than 100 tests)

Remove start and stop

deae3bd

Add a full matrix of versions for test

cdbe97d

Remove the need for a hash file, separate test and reference creation

b53de88

Remove unused dependencies

ea1b220

onigoetz mentioned this pull request Aug 19, 2016

Why is the comparison re-run in the browser ? #254

Closed

onigoetz and others added 4 commits August 19, 2016 09:39

Fix last _.each

3366e33

Remove third parameter on executeCommand, prepare the configuration i…

b142b0f

…n a separate step

Allow to create only part of the references or test only a part of th…

0fd0c89

…e scenarii with "--filter"

Expand/collapse summary list

1f2076f

machowski reviewed Aug 19, 2016
View reviewed changes

garris mentioned this pull request Aug 19, 2016

Revert 241 master #263

Merged

garris closed this Aug 19, 2016

garris reopened this Aug 19, 2016

dmitriyilchgmailcom and others added 3 commits August 29, 2016 10:48

Add ability to copy failed tests labels to clipboard and adjust parti…

88f3f75

…al reference generating with possibility to generate references for specified set of tests.

Merge pull request #2 from garris/version_2_0_onigotez

9edf1e3

Version 2 0 onigotez

Restore the tests to be run in "newdir", lint the code

1f8d7a6

Include the source for all bower components, tweak the report a bit

3fcb1d0

onigoetz changed the title ~~[WIP] Rework of many parts for speed and CI reporting~~ Rework of many parts for speed and CI reporting Aug 29, 2016

Fix the executable finding when backstop is installed without "npm link"

282f82e

garris merged commit 0ba2693 into garris:version_2_0 Aug 30, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework of many parts for speed and CI reporting #259

Rework of many parts for speed and CI reporting #259

onigoetz commented Aug 18, 2016 •

edited

Loading

machowski Aug 19, 2016

onigoetz commented Aug 19, 2016

garris commented Aug 19, 2016

onigoetz commented Aug 19, 2016

garris commented Aug 19, 2016 •

edited

Loading

onigoetz commented Aug 29, 2016

onigoetz commented Aug 29, 2016

onigoetz commented Aug 29, 2016

garris commented Aug 29, 2016

garris commented Aug 30, 2016 •

edited

Loading

Rework of many parts for speed and CI reporting #259

Rework of many parts for speed and CI reporting #259

Conversation

onigoetz commented Aug 18, 2016 • edited Loading

Highlights

Variants

Browser reporting

CLI Reporting

Internal changes

Bugfixes / Regressions

Code Style

Tests

machowski Aug 19, 2016

Choose a reason for hiding this comment

onigoetz commented Aug 19, 2016

garris commented Aug 19, 2016

onigoetz commented Aug 19, 2016

garris commented Aug 19, 2016 • edited Loading

onigoetz commented Aug 29, 2016

onigoetz commented Aug 29, 2016

onigoetz commented Aug 29, 2016

garris commented Aug 29, 2016

garris commented Aug 30, 2016 • edited Loading

onigoetz commented Aug 18, 2016 •

edited

Loading

garris commented Aug 19, 2016 •

edited

Loading

garris commented Aug 30, 2016 •

edited

Loading