Skip to content

Updated phantomjs dependency.#20

Open
Kreozot wants to merge 3 commits into
justim:masterfrom
Kreozot:update-phantom-dep
Open

Updated phantomjs dependency.#20
Kreozot wants to merge 3 commits into
justim:masterfrom
Kreozot:update-phantom-dep

Conversation

@Kreozot

@Kreozot Kreozot commented Apr 12, 2016

Copy link
Copy Markdown
Contributor

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

@Kreozot

Kreozot commented Apr 12, 2016

Copy link
Copy Markdown
Contributor Author

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

Kreozot commented Apr 12, 2016

Copy link
Copy Markdown
Contributor Author

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

@justim

justim commented Apr 13, 2016

Copy link
Copy Markdown
Owner

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