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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃彈 Fix e2e headless mode and make e2e job blocking again #23626

Merged
merged 4 commits into from Aug 1, 2019

Conversation

@estherkim
Copy link
Contributor

commented Jul 31, 2019

Fixes bug where the browser was not running in headless mode even though you passed in the --headless flag.

This should fix the 2 failing e2e tests, so we should reenable the e2e job as blocking on Travis.

@googlebot googlebot added the cla: yes label Jul 31, 2019

estherkim added 2 commits Jul 31, 2019
@cvializ
cvializ approved these changes Aug 1, 2019
Copy link
Contributor

left a comment

LGTM nice finds 馃憦

@rsimha
rsimha approved these changes Aug 1, 2019
@@ -174,7 +187,7 @@ function getFirefoxArgs(config) {
const args = [];

if (config.headless) {
args.push('-headless');
args.push('--headless');

This comment has been minimized.

Copy link
@rsimha

rsimha Aug 1, 2019

Collaborator

Nice catch!

Does this mean we no longer need xvfb In.travis.yml?

This comment has been minimized.

Copy link
@estherkim

estherkim Aug 1, 2019

Author Contributor

Oh maybe! Let me see.

This comment has been minimized.

Copy link
@estherkim

estherkim Aug 1, 2019

Author Contributor

Yup no need for it as long as we use headless. @prateekbh fyi!

@estherkim estherkim merged commit 55894d5 into ampproject:master Aug 1, 2019

9 of 14 checks passed

ampproject/tests/integration (saucelabs) Tests are queued on Travis
Details
ampproject/tests/unit (saucelabs) Tests are queued on Travis
Details
ampproject/owners-check ampproject/owners-check
Details
ampproject/pr-deploy Ready to create a test site.
Details
ampproject/tests/unit (local-changes) Tests were not required
Details
LGTM analysis: JavaScript No new or fixed alerts
Details
ampproject/bundle-size 螖 +0.00KB | no approval necessary
Details
ampproject/tests/e2e (local) 308 tests passed
Details
ampproject/tests/integration (local) 303 tests passed
Details
ampproject/tests/integration (single-pass) 245 tests passed
Details
ampproject/tests/unit (local) 9992 tests passed
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
percy/amphtml Visual review automatically approved, no visual changes found.
Details

@estherkim estherkim deleted the estherkim:fix-e2e-headless branch Aug 1, 2019

thekorn pushed a commit to edelight/amphtml that referenced this pull request Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.