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

Jump to bottom

Reland "[ChromeDriver] Stop using --ignore-certificate-errors" #19377

Merged
merged 1 commit into from Oct 8, 2019
Merged

Reland "[ChromeDriver] Stop using --ignore-certificate-errors" #19377

merged 1 commit into from Oct 8, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 27, 2019 •
edited

This is a reland of 8f486945b4c91f67b76e02bfd8c39b0a9cf08089

Original change's description:

[ChromeDriver] Stop using --ignore-certificate-errors

ChromeDriver was passing --ignore-certificate-errors switch to Chrome,
regardless of whether user wants to ignore certificate errors or not.
Fixing by stop passing this flag to Chrome, and relying on using
Security.setIgnoreCertificateErrors command when user wants to ignore
certificate errors.

Bug: chromedriver:3148
Change-Id: I5e8493f4cca48ebaeb211c95cd7fd645891c8659
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824005
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699890}

Bug: chromedriver:3148
Change-Id: I96ce5e6746af65e01407e7c6c92fed7428d686ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829802
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701614}

wpt-pr-bot
wpt-pr-bot approved these changes Sep 27, 2019
View reviewed changes
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@JohnChen0 @chromium-wpt-export-bot
Reland "[ChromeDriver] Stop using --ignore-certificate-errors"
4ad60c9 
This is a reland of 8f486945b4c91f67b76e02bfd8c39b0a9cf08089

Original change's description:
> [ChromeDriver] Stop using --ignore-certificate-errors
>
> ChromeDriver was passing --ignore-certificate-errors switch to Chrome,
> regardless of whether user wants to ignore certificate errors or not.
> Fixing by stop passing this flag to Chrome, and relying on using
> Security.setIgnoreCertificateErrors command when user wants to ignore
> certificate errors.
>
> Bug: chromedriver:3148
> Change-Id: I5e8493f4cca48ebaeb211c95cd7fd645891c8659
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824005
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Commit-Queue: John Chen <johnchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#699890}

Bug: chromedriver:3148
Change-Id: I96ce5e6746af65e01407e7c6c92fed7428d686ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829802
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701614}
@foolip
Copy link
Member

foolip commented Oct 2, 2019

wpt-firefox-nightly-stability is failing on Taskcluster because of a flaky test:

Test Subtest Results Messages
/webdriver/tests/permissions/set.py OK: 6/10, TIMEOUT: 4/10
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters0] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters1] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters2] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters3] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters4] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters5] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters6] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_invalid_parameters[capabilities0-parameters7] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_non_secure_context[granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_non_secure_context[denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_non_secure_context[prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting0-granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting0-denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting0-prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting1-granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting1-denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting1-prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting2-granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting2-denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state[capabilities0-realmSetting2-prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting0-granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting0-denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting0-prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting1-granted] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting1-denied] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:
/webdriver/tests/permissions/set.py test_set_to_state_cross_realm[capabilities0-realmSetting1-prompt] FAIL: 6/10, MISSING: 4/10 ValueError: Failed to decode response body as JSON:

However, I don't know how the pytest metadata is used and if this could plausibly be related.

@AutomatedTester @andreastt @whimboo can any of you take a look at the changes made here and say if there's cause for concern for Firefox?

@foolip
Copy link
Member

foolip commented Oct 2, 2019

@JohnChen0 FYI that this has gotten stuck in export. We're working on tooling to communicate that back to Gerrit earlier, but for now we deal with it manually.

@whimboo
Copy link
Contributor

whimboo commented Oct 2, 2019

We do not support permissions yet in Firefox. On our side we expect them all to fail. See our own metadata for that particular file: https://searchfox.org/mozilla-central/source/testing/web-platform/meta/webdriver/tests/permissions/set.py.ini

@foolip
Copy link
Member

foolip commented Oct 2, 2019

Thanks @whimboo, I see in https://wpt.fyi/results/webdriver/tests/permissions/set.py?run_id=343820001&run_id=322430003&run_id=322470001&run_id=311240008 these tests are already failing in Firefox on wpt.fyi as well.

I haven't read the tests, but if they make the test sometimes timeout in Firefox I guess you'd need to update your metadata, is that fine? @jgraham how to deal with incoming flakiness nowadays?

I checked for other uses of @pytest.mark.capabilities and found plenty so I think this change is good in a cross-browser sense, but I'll hold off for a few days before admin merging in case someone says otherwise.

@Hexcles
Copy link
Member

Hexcles commented Oct 2, 2019

But why is it flaky?

@foolip
Copy link
Member

foolip commented Oct 7, 2019

I really don't know why it's flaky, or how seriously to investigate a flaky failure mode. @jgraham if we had guidelines for the Ecosystem Infra rotation for how to handle flakiness in Firefox, what do you think it should say about a case like this one?

@foolip foolip mentioned this pull request Oct 8, 2019
Closed
@Hexcles
Copy link
Member

Hexcles commented Oct 8, 2019 •
edited

Anyway, I'm confident that this change does not cause the flakiness, so I'm force-merging it.

@Hexcles Hexcles merged commit cfa91b8 into master Oct 8, 2019
@Hexcles Hexcles deleted the chromium-export-cl-1829802 branch October 8, 2019 20:25
@timovd timovd mentioned this pull request Dec 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wpt-pr-bot wpt-pr-bot wpt-pr-bot approved these changes

Assignees
No one assigned
Labels
chromium-export webdriver wg-testing_browser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@chromium-wpt-export-bot @foolip @whimboo @Hexcles @wpt-pr-bot @JohnChen0