Respect custom --adb-binary for chrome_android-derived products
#33654
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, `chrome_android`-derived products depended on finding a generic `adb` on the `PATH`. The Chromium infrastructure was changed to use `--adb-binary` instead of `PATH` pollution, but the option did not work as documented because wptrunner did not pass the option all the way through.
wpt-pr-bot
added
infra
wpt
wptrunner
WeizhongX
approved these changes
Apr 15, 2022
jonathan-j-lee
added a commit
that referenced
this pull request
Apr 18, 2022
The change #33654 caused `chrome_android`-related runs without `--adb-binary` to fail because the corresponding kwarg was `None` but present. Constructing the kwarg overwrote the kwarg default. This change fixes that bug by using `None` to signal default `adb` usage.
jonathan-j-lee
added a commit
that referenced
this pull request
Apr 18, 2022
The change #33654 caused `chrome_android`-related runs without `--adb-binary` to fail because the corresponding kwarg was `None` but present. Constructing the browser overwrote the kwarg default. This change fixes that bug by using `None` to signal default `adb` usage.
jonathan-j-lee
added a commit
that referenced
this pull request
Apr 18, 2022
The change #33654 caused `chrome_android`-related runs without `--adb-binary` to fail because the corresponding kwarg was `None` but present. Constructing the browser overwrote the kwarg default. This change fixes that bug by using `None` to signal default `adb` usage.
DanielRyanSmith
pushed a commit
to DanielRyanSmith/wpt
that referenced
this pull request
Apr 27, 2022
…eb-platform-tests#33654) Previously, `chrome_android`-derived products depended on finding a generic `adb` on the `PATH`. The Chromium infrastructure was changed to use `--adb-binary` instead of `PATH` pollution, but the option did not work as documented because wptrunner did not pass the option all the way through.
DanielRyanSmith
pushed a commit
to DanielRyanSmith/wpt
that referenced
this pull request
Apr 27, 2022
…d products (web-platform-tests#33669) The change web-platform-tests#33654 caused `chrome_android`-related runs without `--adb-binary` to fail because the corresponding kwarg was `None` but present. Constructing the browser overwrote the kwarg default. This change fixes that bug by using `None` to signal default `adb` usage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously,
chrome_android-derived products depended on finding a genericadbon thePATH. The Chromium infrastructure was changed to use--adb-binaryinstead ofPATHpollution, but the option did not work as documented because wptrunner did not pass the option all the way through.Context: https://chromium-review.googlesource.com/c/chromium/src/+/3578461. The Android wpt trybot I was using masked the bug: https://ci.chromium.org/ui/p/chromium/builders/try/android-weblayer-pie-x86-wpt-smoketest/2036/overview
I've tried this change in
chromium/srcin a shell withoutadbinPATHand verified that this change fixes the immediate error forchrome_android,android_webview, andandroid_weblayer. Looking atgrep -R "[\"']adb[\"']" --exclude-dir='*venv' tools, I think there are no more instances of hardcoded genericadb.