-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Pass server config to WebDriver via a file instead of an env variable. #28834
Conversation
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.
Thanks!
Test failure is |
Hmm, "Azure Pipelines (tools/wpt/ tests: macOS + Python 3.6)" failed in a novel way:
https://raw.githubusercontent.com/Homebrew/homebrew-cask-versions/master/Casks/firefox-nightly.rb exists, but perhaps there was a network error. I'll close and reopen to see if it was a fluke. We could probably install Firefox using |
Different failure this time in "Azure Pipelines (tools/wpt/ tests: macOS + Python 3.6)": a flaky manifest test, it seems.
Trying to close and re-open once more. |
@letitz also try rebasing to be sure you're testing the latest CI configuration. |
The |
Tried rebasing per @foolip's suggestion. |
@letitz it looks like flake8 is still failing, did you accidental revert that fix? |
Oh, I think I totally did. Sorry! Not very familiar with the PR workflow yet, especially around how to handle commits added to PRs by others. Fixing... |
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.
CI is still running, but changes look good!
This change unblocks importer, where webdriver tests are failing due to changes in web-platform-tests/wpt#28834 Change-Id: Ia07ed877aa808da41a49260900a390c87e9c5521 No-Export: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2890259 Reviewed-by: Jason Chase <[email protected]> Reviewed-by: John Chen <[email protected]> Commit-Queue: Kyle Ju <[email protected]> Cr-Commit-Position: refs/heads/master@{#882198}
This change unblocks importer, where webdriver tests are failing due to changes in web-platform-tests/wpt#28834 Change-Id: Ia07ed877aa808da41a49260900a390c87e9c5521 No-Export: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2890259 Reviewed-by: Jason Chase <[email protected]> Reviewed-by: John Chen <[email protected]> Commit-Queue: Kyle Ju <[email protected]> Cr-Commit-Position: refs/heads/master@{#882198}
This caused a regression for several tests across browsers. We are currently investigating in https://bugzilla.mozilla.org/show_bug.cgi?id=1744249. |
This change unblocks importer, where webdriver tests are failing due to changes in web-platform-tests/wpt#28834 Change-Id: Ia07ed877aa808da41a49260900a390c87e9c5521 No-Export: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2890259 Reviewed-by: Jason Chase <[email protected]> Reviewed-by: John Chen <[email protected]> Commit-Queue: Kyle Ju <[email protected]> Cr-Commit-Position: refs/heads/master@{#882198} NOKEYCHECK=True GitOrigin-RevId: 1f1ca91d06821cb5dd2fe057d4efa4fc572ee158
Instead of serializing the whole config in an environment variable, I amended the code to dump the config as JSON in a temporary file (the tests are conveniently executed against a temporary directory) and pass the file path in the environment variable instead.
This should avoid failures due to the 16-bit length limit being exceeded for good. In other words, no more issues like #21705 (comment) and #12461 :) This also removes the need to special-case origin-policy subdomains.
This change was extracted from #28768 per @jgraham's suggestion.