[web] Fixing launching Safari. This should solve the LUCI issue #16590
Conversation
lib/web_ui/dev/safari.dart
Outdated
| // to open, that instance to wait for opening the url until Safari launches, | ||
| // provide Safari bundles identifier. | ||
| // For more details run `man open` on MacOS. | ||
| // TODO(nurhan): explore implementing this part using Apple Script or |
There was a problem hiding this comment.
Consider creating an issue and linking to it from here.
|
Launching the tests succeeded in LUCI. |
nturgut
force-pushed
the
fix_safari_launch
branch
from
083747f to
595f851
Compare
| // provide Safari bundles identifier. | ||
| // The details copied from `man open` on MacOS. | ||
| // TODO(nurhan): https://github.com/flutter/flutter/issues/50809 | ||
| var process = await Process.start(installation.executable, [ |
There was a problem hiding this comment.
The Browser class expects that the process object returned from this function is the browser process, or at least some proxy of it. However, this would return the open process that quits immediately. I have a feeling this will become a thorn in future evolution of the Browser class. For example, this will prevent us from being able to implement logic that watches the health of the browser process, such as premature termination (examples).
Having said that, I don't have an immediate solution to this, other than what we talked about offline, i.e. use WebDriver to control launching of browsers. If I'm reading the w3c spec correctly, Selenium is not necessary to use it (see WebDriver Protocol). So please, treat this comment as non-blocking. I'll file a separate issue for us to investigate a WebDriver solution.
|
I merging the change since it is related to safari engine unit test tooling. Hopefully will run luci-engine and we will get a green. |
* c0549fb Roll buildroot. (flutter/engine#16613) * 8b0b649 improve surface state assert error messages (flutter/engine#16595) * cd77e78 Fix drawRRect failure when shader is specified (flutter/engine#16601) * fe63094 [web] Handle alignment correctly in Paragraph.getPositionForOffset (flutter/engine#16569) * 65d1126 [web] Fixing launching Safari. This should solve the LUCI issue (flutter/engine#16590) * f88f7df [web] Unskip tests that are already passing in Safari (flutter/engine#16567) * 594f660 [shell tests] Integrate Vulkan with Shell Tests * 400ed7c Revert "[shell tests] Integrate Vulkan with Shell Tests" * 15e7f51 Implement Path extractPath, tangent APIs (flutter/engine#16599) * 4941ff7 Remove usage of Dart_AllocateWithNativeFields from tonic (flutter/engine#16588) * bb01cb7 Roll fuchsia/sdk/core/linux-amd64 from Bmq1m... to J-_s6... (flutter/engine#16592)
…ter#16590) * Fixing launching Safari. This should solve the LUCI issue * more comments and linking the issue
…ue (flutter#16590)" This reverts commit 46ab65a.
Changing the way we are launching Safari for tests.
So far we were using the method from dart-sdk/test to open safari browser, on the other hand it looks like this method become obsolete with the new MacOs version with the increasing file security. It became an issue in other projects as well. We applied one of the solutions proposed here: karma-runner/karma-safari-launcher#29 (comment)