-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
Fix headless chrome in CircleCI #1636
Fix headless chrome in CircleCI #1636
Conversation
* reverts parts of sinonjs#1629
.circleci/config.yml
Outdated
# HACK: modify mochify to pass args to chrome to allow it to run as root | ||
# --no-sandbox --disable-setuid-sandbox | ||
# See https://github.com/mantoni/mochify.js/issues/162 | ||
sed -i "s#'--allow-insecure-localhost'#'--allow-insecure-localhost', '--no-sandbox', '--disable-setuid-sandbox'#" node_modules/mochify/lib/chromium.js && echo "modified mochify/lib/chromium.js"; |
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 is gross but a necessary evil for now. This is the result without the hack.
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.
We'll get this fixed in Mochify.
@mantoni merged the fix to mochify (mantoni/mochify.js#163) and I tested it and everything is working. Just need an npm version of mochify to point to wrap this puppy up. (Thanks for the quick merge.) |
@fearphage Already there. Just published |
You're quick! |
Out of curiosity, why did you go with a separate download of chromium instead of relying on the puppeteer install script? |
I read it was necessary (when researching #1629). I didn't even try the default installed version. |
1e93b17
to
d078af9
Compare
Hm, I think it now used the cache instead of doing a new npm install and therefore doesn't find Chrome. |
switched v4 and v8 to using alpine images
I'm done fiddling with this one. Review/merge away.
Did a fresh npm install and that build failed. |
@fearphage Thank for trying. Hopefully this gets resolved with a future release of puppeteer. |
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.
I think this is good to merge.
This re-enables headless chrome testing in CircleCI. It has one huge hack in it right now (manually editing a dependency). I think perhaps we should wait on merging this for a cleaner solution. I filed an issue with mochify (mantoni/mochify.js#162) (cc @mantoni).
This reverts small amounts of #1629.