Fix visibility of web server device when Chrome is not available #40757
Conversation
|
I pushed some changes to the workflow test to remove the assumption that it can't list devices if there's no Chrome - however I'm not sure if this is the best way to change it (it's not clear if returning true for some flags when |
Codecov Report
@@ Coverage Diff @@
## master #40757 +/- ##
==========================================
- Coverage 59.56% 59.1% -0.47%
==========================================
Files 192 192
Lines 18683 18683
==========================================
- Hits 11129 11042 -87
- Misses 7554 7641 +87
Continue to review full report at Codecov.
|
jonahwilliams
left a comment
There was a problem hiding this comment.
The flags on Workflow are a bit weird, I agree. This seems correct though, its similar to an Android device discoverer having a valid SDK but no connected device.
LGTM
| Future<List<Device>> pollingGetDevices() async { | ||
| return <Device>[ | ||
| _webDevice, | ||
| if (_chromeIsAvailable) |
There was a problem hiding this comment.
This seems reasonable
|
@jonahwilliams test failures were due to the tests relying on Chrome actually being on the underlying machine (because there were two checks - that we can find it, and that we can run it). I've pushed a change that moves |
|
@jonahwilliams I'm out the office shortly for a week so if you're happy with the extra change I pushed, feel free to merge in my absence (@domesticmouse may be happy having the fix to try out :-)) |
|
@jonahwilliams ping! Still looking for confirmation of the changes I pushed (2c31a79) to resolve the test issues. |
…tter#40757) * Fix visbility of web server device when Chrome is not available * Add tests * Update workflow test * Fix tests to not rely on Chrome being on the underlying machine
This removes the requirement of finding Chrome for all web devices, but does check it before providing the Chrome device.
Fixes #40814.