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

Puppeteer fix for creating blank (off page) screenshots #901

Merged
merged 3 commits into from
Oct 24, 2018

Conversation

thumpmaster
Copy link
Contributor

Following to the conversation from this pull request https://github.com/garris/BackstopJS/pull/808/files, I've added the puppeteerOffscreenCaptureFix feature flag.

Could you please consider merging this as soon as you can? Thanks for this.

@cactusa
Copy link
Contributor

cactusa commented Oct 24, 2018

HI @garris this is one of my colleagues finishing the PR I started months ago. As previously discussed this wraps the fix in a feature flag so it doesn't break any of you existing tests until you have the time to look into it in depth. This should enable us to run backstop in our CI pipeline in the meantime. Thank you again! Let us know if you's like us to make any changes to the PR.

@garris
Copy link
Owner

garris commented Oct 24, 2018

Thank you for the commit! I will attempt to push this today.

@cactusa
Copy link
Contributor

cactusa commented Oct 24, 2018

Thank you very much!

@garris
Copy link
Owner

garris commented Oct 24, 2018

This is published to npm with the @beta tag. This branch also has some refactored code on it. Please validate when you get a chance -- if it's working for you I can push to latest. Cheers!

npm install backstopjs@beta (with optional -g flag)

@cactusa
Copy link
Contributor

cactusa commented Oct 25, 2018

Ok would that beta version also work for Docker? Would Docker be pointed to that beta version specifically or would it only work with the latest backstop version?

@garris
Copy link
Owner

garris commented Oct 25, 2018

I expect it would work.

@cactusa
Copy link
Contributor

cactusa commented Nov 9, 2018

We have tested this and it doesn't work in docker. It works just fine when it's not run in docker. I suspect that the docker image still pulls the latest version of backstop and ignores the beta branch.

@thumpmaster
Copy link
Contributor Author

@garris Running the tests outside of the docker works fine.

@garris
Copy link
Owner

garris commented Nov 12, 2018

Hi guys -- I think I know the issue. I just pushed the tag with your fix to github. I had forgotten to do that before.

When BackstopJS is run with the --docker flag, backstop should ask docker to pull down the docker image corresponding to the same BackstopJS version that is running locally. This should be the case even when running @beta. Unfortunately-- I forgot to publish the tag with your fix before. Please give it another try and please send terminal output if it still doesn't work. Cheers!

@cactusa
Copy link
Contributor

cactusa commented Nov 12, 2018

Thanks for that, will get back to you tomorrow!

@cactusa
Copy link
Contributor

cactusa commented Nov 13, 2018

Everything has been tested and works as expected on the beta version with or without docker. Would you be able to give us an estimate of when you'll be able to make an official release of this?

@garris
Copy link
Owner

garris commented Nov 13, 2018

Fantastic! I will do ASAP. Hopefully today or tomorrow AM.

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.

3 participants