-
Notifications
You must be signed in to change notification settings - Fork 106
Screenshotting support #109
base: master
Are you sure you want to change the base?
Conversation
|
||
To change the resolution of the viewport: | ||
```js | ||
console.log(JSON.stringify({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can may now configure a viewport size in available in newer versions of the grunt-lib-phantomjs dependency. So, maybe bump the dependency version?
https://github.com/gruntjs/grunt-lib-phantomjs/blob/master/Gruntfile.js#L74-78
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robcolburn we needed to be able to keep changing the viewport during testing - especially with our responsive Sky.com websites that cater for mobile, tablet and desktop viewports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Makes sense. Having both (grunt) declarative and programmatic control of viewport is ideal.
ping |
Thanks for the ping :) I will make the changes and get back to you guys.
|
* 'master' of https://github.com/kmiyashiro/grunt-mocha: Add test for options.page Add test object to page example Update README.md with phantomjs page settings info Merge phantomjs page settings from Grunt config Bump 0.4.11 Fixes kmiyashiro#119. Fix mislocated reporter in README dest example Indicated correct default reporter in documentation Remove `tasks` from readme Make Growl notifications on success an option Add onResourceTimeout handler. Add onResourceError handler.
@robcolburn I have pulled upstream and fixed the |
page.render(json.filename); | ||
} | ||
} catch(err) { | ||
sendMessage('console', 'Console message could not be parsed.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This catch statement seems like it could get rather chatty with application code also using the console. Maybe scrap it or put the output behind a specific flag?
@bjfletcher, this looks good to me, though I'm not a maintainer on this module. Just hoping to help out the process for a someone who is. |
Is there a reason this hasn't been merged yet? This would be really useful to me. |
@pimterry have you tried using it and seeing if it actually works how you think it would? |
Nope, for various reasons (this, and some PhantomJS issues that would need me to get PhantomJS 2 working, which seems to be pretty impractical) I've moved to Karma + Chrome instead, which means I can just look at the browser myself. There are definitely other people using it though, as the fork's been published to NPM as a standalone release on NPM in the meantime: https://www.npmjs.com/package/grunt-mocha-screenshot. |
We used this at Sky in a couple of projects in production for visual regression testing. This was followed in the build process by a visual comparison step https://github.com/bjfletcher/grunt-screenshot-compare The build process would proceed only if there was no visual regression. |
I have added API for creating screenshots and for changing the viewpoint resolution. The API from Mocha to PhantomJS is achieved using specific console.log's that PhantomJS listens to. I have tested this and working well.