Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual regression testing #31

Closed
luboskmetko opened this issue Aug 18, 2016 · 25 comments
Closed

Visual regression testing #31

luboskmetko opened this issue Aug 18, 2016 · 25 comments

Comments

@luboskmetko
Copy link
Member

xfiveco/generator-xh#122

@thymikee
Copy link
Contributor

I've just bumped into this article https://css-tricks.com/automating-css-regression-testing/
It was mentioned in xfiveco/generator-xh#122 but there was major update. I think it's worth to consider this.

What bugs me is that we would need to store the screenshots in the repo, which will add couple of Megs. But then again, it's not that much of an overhead, isn't it?

@danekszy
Copy link
Contributor

I'm all in for this. 👍
Though I do think that this should be a) optional and/or b) also possible to use in manual mode...
Visual regression testing isn't necessary during active development (initial sprint etc). but proves super helpful in case of refactoring or feature adding and maintenance.
Perhaps manual snapshot triggering task would cover such use cases and help us save some storage as well.

Also, does the Chisel store page index in json as of 0.2.2? I proposed this specifically for such features, which need to iterate over all/some views (pages).

@luboskmetko
Copy link
Member Author

Looks cool, nice to see it's an active project and the progress they've made.

What if screenshots are git ignored and created on previewized too (like dist is built)? Haven't checked it in detail yet how time consuming is this task but agree with Daniel, that it could be triggered manually (in such case maybe with some post commit hook on previewized)

@luboskmetko
Copy link
Member Author

@danekszy you mean pages list? yes, it is stored in .yo-rc.json. Probably we could extend that list to allow marking certain pages as done or in progress.

@thymikee
Copy link
Contributor

@danekszy I thought on having this on build run. @Miriloth is investigating into the tool, tomorrow I'll run my own tests on this.

@luboskmetko I think it works this way: get a screenshot => store it => get another screenshot in another commit => compare.

@danekszy
Copy link
Contributor

danekszy commented Sep 19, 2016

@luboskmetko Great to hear that. Does out project index (twig as I discovered) use that json? Can we expand it into page object with many properties, or does Yo enforce it to be an array of strings?
In our brainstorm I remember considering adding page-specific template variables there (like page title etc.).

@thymikee These are the use cases that I considered as well. But like I mentioned, I'd like to be able to manually trigger this before commiting in order to see if I introduced any unwanted changes. This would require us to decouple it from any commit/build-task related logic.
Another issue is that I find myself running build rather often, so that image additions and renames are moved over to dist... But I admit, I can't tell if that's solved in Chisel already.

@thymikee
Copy link
Contributor

This would be just another gulp task, so it's not a problem to run it
separately. I also see it as an option for a generator. A thing to consider
— setup githooks before pushing to the repo.

19 wrz 2016 16:06 "Daniel Szymanek" notifications@github.com napisał(a):

@luboskmetko https://github.com/luboskmetko Great to hear that. Does
out project index (twig as I discovered) use that json? Can we expand it
into page object with many properties, or does Yo enforce it to be an array
of strings?

@thymikee https://github.com/thymikee These are the use cases that I
considered as well. But like I mentioned, I'd like to be able to manually
trigger this before commiting in order to see if I introduced any unwanted
changes. This would require us to decouple it from any commit/build-task
related logic.
Another issue is that I find myself running build rather often, so that
image additions and renames are moved over to dist... But I admit, I can't
tell if that's solved in Chisel already.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#31 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE3rIlAcINsQxom_wdldr4bb_rjfnaC4ks5qrpbtgaJpZM4JnM-F
.

@danekszy
Copy link
Contributor

That's what I figured. What I meant by decoupling is logical separation - so for example not naming snapshots/screenshots after commit hash etc.

@luboskmetko
Copy link
Member Author

@danekszy project index is generated with page generator https://github.com/xfiveco/generator-chisel/blob/master/generators/page/index.js and uses index/project-index.html as a template. I think each page can be extended with more properties but we would also have to regenerate the project index then.

jakub300 added a commit to jakub300/generator-chisel that referenced this issue Sep 27, 2016
@jakub300
Copy link
Collaborator

Unfortunately most solutions are based on PhantomJS which is not up to date on newest Webkit features. On the other hand Google is actively working on headless Chromium. It looks like the best option today to wait and see where Chromium work is going.

@thymikee
Copy link
Contributor

Closing for now. Let's reopen this when headless chrome gets stable

@danekszy
Copy link
Contributor

danekszy commented Sep 27, 2016

I wouldn't throw away this just yet...
Visual regression testing is useful mainly for layout changes (and not some design details like shadows etc.), so perhaps we should look deeper into this.
Depending on how much time such research would take, we could look into what features are missing exactly.
If its sth like flexbox, that's a bummer, but I believe some big brands are using VRT (Guardian is the one I remember), so perhaps this isn't as bad as it looks.

Later we could just switch to Chrome (which probably won't be perfect either, bc it takes up a lot of RAM)...
What do you think guys?

@thymikee
Copy link
Contributor

@danekszy It's flexbox mostly.

@danekszy
Copy link
Contributor

Damn it, that looks like a deal breaker here...

@jakub300
Copy link
Collaborator

Note for the future: there is also Wraith made and used by BBC: https://github.com/BBC-News/wraith

@danekszy
Copy link
Contributor

What about looking into Phantom.js v2?
http://stackoverflow.com/questions/28827361/does-phantomjs-support-bayeux-or-websockets/28827509#28827509
Looks like it supports most features.
Wondering if porting some tools/plugins to this would be troublesome...

@jakub300
Copy link
Collaborator

When I tested BackstopJS I had problems with flexbox and I think it used newest Phantom (2.1), but I'll check it one more time tomorrow.

@jakub300
Copy link
Collaborator

Here is simple page with flexbox, and how it looks on PhantomJS 2.1.1:
image

@danekszy
Copy link
Contributor

danekszy commented Nov 8, 2016

Just FYI...
https://github.com/dhamaniasad/HeadlessBrowsers

We might need to take another look at this feature, if we're done with more important stuff...

@danekszy
Copy link
Contributor

danekszy commented Jan 9, 2017

Another update: There's PhantomJS 2.5 going to be released soon and its based on QtWebkit.
I think they've made some progress in regards to compatibility/feature implementation...
https://groups.google.com/forum/#!topic/phantomjs/AefOuwkgBh0

@jakub300
Copy link
Collaborator

PhantomJS maintainer is stepping down expecting Headless Chrome to take over: https://groups.google.com/d/msg/phantomjs/9aI5d-LDuNE/5Z3SMZrqAQAJ

@danekszy
Copy link
Contributor

danekszy commented May 2, 2017

Here's a recipe for taking screenshots with Headless Chrome:
https://developers.google.com/web/updates/2017/04/headless-chrome
It's coming to stable in next release and is now available in Beta channel.

@jakub300
Copy link
Collaborator

jakub300 commented May 2, 2017

Yeah, I've already used it in one project for testing. It's also worth noting that integration(?) between PhantomJS and Headless Chome is in the talks: ariya/phantomjs#14954.

@danekszy
Copy link
Contributor

danekszy commented May 2, 2017

Awesome, looks good. 😄 Any observations so far? Is it stable? Any differences in render?

@jakub300
Copy link
Collaborator

jakub300 commented May 2, 2017

It worked great, one problem was (that I also noticed when using Chrome dev tools normally) that switching between desktop and mobile mode not always rerenders the page. As situation develops question remains whether we should use Chrome Headless directly or wait for some phantom-like integration/support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants