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

Updated phantomjs dependency. #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Kreozot
Copy link
Contributor

@Kreozot Kreozot commented Apr 12, 2016

Plus added workaround for regression in phantomjs 2.0.0 (on Windows).

@Kreozot
Copy link
Contributor Author

Kreozot commented Apr 12, 2016

It's strange that test has failed. I think, it depends on platform and phantom version. If you compare actual and expected png-s - they completely the same. But for some reason bytes of these files is not the same.
I was testing master branch before and it failed. Then i tried to fix it by 84de565 commit - it fix tests on my machine but not on Travis.

@Kreozot
Copy link
Contributor Author

Kreozot commented Apr 12, 2016

I just put files generated on linux and tests has passed.

@justim
Copy link
Owner

justim commented Apr 13, 2016

The different expected images are not unexpected, different versions of PhantomJS yield different output, as long as they look the same, we're good.

Just using the following works just fine on a mac.

var sourceUrl = 'file:///' + source;

Does this also work on linux?

If it does, we don't need the workaround and we can just use the file:/// prefix.

And lastly, the path.resolve introduces an issue that was fixed a little while back (#17), so we need a better solution for this.

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

Successfully merging this pull request may close these issues.

2 participants