Combine the WebDriver setup process into WebDriverService and make it easier to reuse #474

omochi · 2024-05-26T06:24:50Z

背景

#473 のために、WebDriver を利用したい。
WebDriver の実装は WebDriverClient.swift にクライアント実装、
CartonFrontendTestCommand.swift にドライバの選択処理などが書かれている。

提案

WebDriver関連の実装をモジュールにまとめて、
テストなどから再利用しやすいようにする。

`WebDriverHTTPClient` の公開

WebDriverClient において、
curl が使える状況では引数で受け取った URLSession を捨てるようになっていた。

また、 curl と URLSession の抽象化のために WebDriverHTTPClient を定義していた。
URLSession はこれに直接準拠していた。

WebDriverClient がHTTP通信に使う依存物は、
ユーザー側から注入をコントロールできるようにした方が良い。
そのほうが、個別の実装をテストしたり、
自動選択したりする挙動を使う意図が明確なコードを書くことができる。

しかし、 WebDriverHTTPClient のメソッドは、
URLSession の既存のメソッドと紛らわしいし、
仕様がこの用途に特化しているため、他の場所から使われたくないため、
URLSession への準拠させたくはない。

そこで、 URLSessionWebDriverHTTPClient というアダプタを用意する。
また、 Curl 型も CurlWebDriverHTTPClient にリネームする。

さらに、従来の選択挙動は WebDriverHTTPClients.find として利用可能にする。

Linux の対応

URLSessionWebDriverHTTPClient は Linux では全く動かないことがわかった。
既存の実装も動いてなかったらしい。
そこで、これが動かないことをコメントしつつ、
#if os でLinuxでは完全に存在を消す。

以下で起票された。
apple/swift-corelibs-foundation#4965

`WebDriverService` の実装

CartonFrontendTestCommand において３種類のWebDriver利用パターンがあり、
そのうち2種類は終了時に専用のクリーンアップ処理が必要になっている。

そこで、この概念を WebDriverService として抽象化して、
3種類の実装を2つの具体型の RemoteWebDriverService と CommandWebDriverService として定義し直す。
WebDriverService はすでに起動した web driver を表現していて、
dispose メソッドでシャットダウンを行う。
また、(extensionで定義された) client メソッドに
WebDriverHTTPClient を渡すことで、
セッションの初期化が終了し通信できる状態になったクライアントを返す。

また、従来のドライバ選択ロジックは WebDriverServices.find として利用可能にする。

`withRetry` の公開

WebDriverの既存ロジックにリトライ施行があったが、
CommandTestHelper の withRetry で同じ挙動を再現できるので、
これを CartonHelpers に移動して共通化する。

これはリトライ時のメッセージも出すので、
よりユーザが状況を把握しやすい。
以下は実例のテストの出力で、safaridriver + curlの場合に、
curlの呼び出しが早すぎて一回失敗していることがわかる

Test Suite 'Selected tests' started at 2024-05-26 15:24:29.299.
Test Suite 'WebDriverTests.xctest' started at 2024-05-26 15:24:29.299.
Test Suite 'WebDriverClientTests' started at 2024-05-26 15:24:29.299.
Test Case '-[WebDriverTests.WebDriverClientTests testGotoCurl]' started.
- checking WebDriver endpoint: �[36m�[1mWEBDRIVER_REMOTE_URL
�[0m- checking WebDriver executable: �[36m�[1mWEBDRIVER_PATH
�[0m- checking WebDriver executable in PATH: �[36m�[1mchromedriver, geckodriver, safaridriver, msedgedriver
�[0mLaunch WebDriver executable: �[36m�[1m/usr/bin/safaridriver
�[0mattempt 1 failed: /usr/bin/curl failed with status 7., retrying...
Test Case '-[WebDriverTests.WebDriverClientTests testGotoCurl]' passed (2.276 seconds).
Test Suite 'WebDriverClientTests' passed at 2024-05-26 15:24:31.576.
	 Executed 1 test, with 0 failures (0 unexpected) in 2.276 (2.277) seconds
Test Suite 'WebDriverTests.xctest' passed at 2024-05-26 15:24:31.576.
	 Executed 1 test, with 0 failures (0 unexpected) in 2.276 (2.278) seconds
Test Suite 'Selected tests' passed at 2024-05-26 15:24:31.577.
	 Executed 1 test, with 0 failures (0 unexpected) in 2.276 (2.278) seconds
Program ended with exit code: 0

`WebDriver` モジュールへの名前変更

WebDriverService なども含まれたので、
WebDriverClient モジュールを WebDriver モジュールに名前変更する

omochi

self comment

omochi · 2024-05-26T06:47:32Z

Sources/CartonHelpers/Retry.swift

@@ -0,0 +1,25 @@
+public func withRetry<R>(


コピペで移動してきた

omochi · 2024-05-26T06:48:15Z

Sources/WebDriver/CommandWebDriverService.swift

+
+public struct CommandWebDriverService: WebDriverService {
+  private static func findAvailablePort() async throws -> SocketAddress {
+    let bootstrap = ServerBootstrap(group: .singletonMultiThreadedEventLoopGroup)


ここの EventLoopGroup だけ NIOPosixの提供するシングルトンに変えた

他はコピペ

omochi · 2024-05-26T06:48:43Z

Sources/WebDriver/CommandWebDriverService.swift

+  private static func launchDriver(
+    terminal: InteractiveWriter,
+    executablePath: String
+  ) async throws -> (URL, CartonHelpers.Process) {


Processを返すように変更する

omochi · 2024-05-26T06:49:23Z

Sources/WebDriver/CommandWebDriverService.swift

+  public var process: CartonHelpers.Process
+
+  public func dispose() {
+    process.signal(SIGKILL)


このサービスはコマンドで起動するのでdisposeでKILLするためにProcessを持つ

omochi · 2024-05-26T06:49:54Z

Sources/WebDriver/CurlWebDriverHTTPClient.swift

+    guard process.terminationStatus == 0 else {
+      let body: String? = responseBody.map { String(decoding: $0, as: UTF8.self) }
+
+      throw WebDriverError.curlError(


ここエラーの内容をリッチにした。

omochi · 2024-05-26T06:50:17Z

Sources/WebDriver/RemoteWebDriverService.swift

+
+  public var endpoint: URL
+
+  public func dispose() {}


これはリモートのURLを持つだけなので空

omochi · 2024-05-26T06:53:22Z

Tests/WebDriverTests/WebDriverClientTests.swift

-  func checkRemoteURL() throws -> URL {
-    guard let value = ProcessInfo.processInfo.environment["WEBDRIVER_REMOTE_URL"] else {
-      throw XCTSkip("Skip WebDriver tests due to no WEBDRIVER_REMOTE_URL env var")
+  func testGotoURLSession() async throws {


このテストケースの目的は、
クライアント実装が動作するか確かめることなので、
URLSessionとCurlの両方を確かめたい。

それぞれテストケースとして分離して、明示的に渡すことでテストする。

ここではクライアントのテストがしたいので、
どの方式で Web Driver を起動しているかは関心がないため、
WEBDRIVER_REMOTE_URL を参照する挙動は変更して、
carton test コマンドと同じロジックで利用可能な Web Driver を検索させるようにする。

それはそれとして、
様々な WebDriver 実装ごとのテストを追加した方が良い。
このパッチではやらない。

omochi · 2024-05-26T06:56:45Z

CIのLinuxのエラー

- checking WebDriver endpoint: WEBDRIVER_REMOTE_URL
- checking WebDriver executable:
WEBDRIVER_PATH
- checking WebDriver executable in PATH: chromedriver, geckodriver, safaridriver, msedgedriver
Launch WebDriver executable: /usr/bin/chromedriver
attempt 1 failed: /usr/bin/curl failed with status 7., retrying...
attempt 2 failed: /usr/bin/curl failed with status 7., retrying...
Test Case 'WebDriverClientTests.testGotoCurl' passed (15.176 seconds)
Test Case 'WebDriverClientTests.testGotoURLSession' started at 2024-05-26 06:53:22.336
- checking WebDriver endpoint: WEBDRIVER_REMOTE_URL
- checking WebDriver executable: WEBDRIVER_PATH
- checking WebDriver executable in PATH: chromedriver, geckodriver, safaridriver, msedgedriver
Launch WebDriver executable: /usr/bin/chromedriver
attempt 1 failed: Error Domain=NSURLErrorDomain Code=-1004 "(null)", retrying...
attempt 2 failed: Error Domain=NSURLErrorDomain Code=-1 "(null)", retrying...
<EXPR>:0: error: WebDriverClientTests.testGotoURLSession : threw error "Error Domain=NSURLErrorDomain Code=-1 "(null)""
Test Case 'WebDriverClientTests.testGotoURLSession' failed (2.255 seconds)

curlの方も3度目の正直でギリギリ通過してるので、
リトライ条件がシビアすぎるのかも？

omochi · 2024-05-26T12:31:06Z

Test Case 'WebDriverClientTests.testGotoURLSession' started at 2024-05-26 12:19:27.935
- checking WebDriver endpoint: WEBDRIVER_REMOTE_URL
- checking WebDriver executable: WEBDRIVER_PATH
- checking WebDriver executable in PATH: chromedriver, geckodriver, safaridriver, msedgedriver
Launch WebDriver executable: /usr/bin/chromedriver
attempt 1/5 failed: Error Domain=NSURLErrorDomain Code=-1 "(null)", retrying in 10.0 seconds...
attempt 2/5 failed: Error Domain=NSURLErrorDomain Code=-1 "(null)", retrying in 10.0 seconds...
attempt 3/5 failed: Error Domain=NSURLErrorDomain Code=-1 "(null)", retrying in 10.0 seconds...
attempt 4/5 failed: Error Domain=NSURLErrorDomain Code=-1 "(null)", retrying in 10.0 seconds...
<EXPR>:0: error: WebDriverClientTests.testGotoURLSession : threw error "Error Domain=NSURLErrorDomain Code=-1 "(null)""

ふ〜む？

omochi · 2024-05-26T13:04:34Z

手元で捕捉

omochi · 2024-05-26T14:17:52Z

生まれてからずっと壊れていたらしくて、corelibs-foundation内部のバグの可能性が高いので、この問題は見送る。
https://discord.com/channels/291054398077927425/383442648012423179/1244286842938064897

omochi · 2024-05-26T14:19:30Z

Sources/WebDriver/URLSessionWebDriverHTTPClient.swift

+
+#else
+
+// Due to a broken URLSession in swift-corelibs-foundation, this class cannot be used on Linux.


Linuxでは動かないという情報を残しておく

omochi · 2024-05-26T23:58:24Z

@kateinoigakukun まとまったので確認をお願いします

kateinoigakukun · 2024-05-27T01:16:08Z

Sources/WebDriver/WebDriverHTTPClient.swift

+      return curl
+    }
+
+    #if os(Linux)


URLSession is broken on not only Linux but also all platforms that use corelibs-foundation. So I think #if canImport(FoundationNetworking) would be the correct condition.

なるほど。後で直します。

kateinoigakukun

Thanks!

omochi added 4 commits May 26, 2024 14:29

refactor WebDriver module

3b00c68

WebDriverService

87aa425

FoundationNetworking

5d33130

import Foundation

c63bb8a

omochi commented May 26, 2024

View reviewed changes

adjust retry configuration

caa74ec

remove URLSession client on Linux

def52ac

omochi commented May 26, 2024

View reviewed changes

omochi marked this pull request as ready for review May 26, 2024 23:56

omochi changed the title ~~[WIP] WebDriverのセットアップ処理をWebDriverServiceにまとめて再利用しやすくする~~ WebDriverのセットアップ処理をWebDriverServiceにまとめて再利用しやすくする May 26, 2024

kateinoigakukun reviewed May 27, 2024

View reviewed changes

omochi added 3 commits May 28, 2024 08:43

check FoundationNetworking instead of Linux

aefec30

combine pound if

67ed784

remove unused import

648dba1

kateinoigakukun approved these changes May 28, 2024

View reviewed changes

kateinoigakukun changed the title ~~WebDriverのセットアップ処理をWebDriverServiceにまとめて再利用しやすくする~~ Combine the WebDriver setup process into WebDriverService and make it easier to reuse May 28, 2024

kateinoigakukun merged commit f7d0b56 into swiftwasm:main May 28, 2024
4 checks passed

omochi deleted the webdriver-module branch May 28, 2024 01:55

omochi mentioned this pull request May 28, 2024

Check the output of wasm in dev subcommand test #473

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine the WebDriver setup process into WebDriverService and make it easier to reuse #474

Combine the WebDriver setup process into WebDriverService and make it easier to reuse #474

omochi commented May 26, 2024 •

edited

Loading

omochi left a comment

omochi May 26, 2024

omochi May 26, 2024

omochi May 26, 2024

omochi May 26, 2024

omochi May 26, 2024

omochi May 26, 2024

omochi May 26, 2024

omochi commented May 26, 2024

omochi commented May 26, 2024

omochi commented May 26, 2024

omochi commented May 26, 2024

omochi May 26, 2024

omochi commented May 26, 2024

kateinoigakukun May 27, 2024

omochi May 27, 2024 •

edited

Loading

kateinoigakukun left a comment


		#else

		// Due to a broken URLSession in swift-corelibs-foundation, this class cannot be used on Linux.

Combine the WebDriver setup process into WebDriverService and make it easier to reuse #474

Combine the WebDriver setup process into WebDriverService and make it easier to reuse #474

Conversation

omochi commented May 26, 2024 • edited Loading

背景

提案

WebDriverHTTPClient の公開

Linux の対応

WebDriverService の実装

withRetry の公開

WebDriver モジュールへの名前変更

omochi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omochi commented May 26, 2024

omochi commented May 26, 2024

omochi commented May 26, 2024

omochi commented May 26, 2024

Choose a reason for hiding this comment

omochi commented May 26, 2024

Choose a reason for hiding this comment

omochi May 27, 2024 • edited Loading

Choose a reason for hiding this comment

kateinoigakukun left a comment

Choose a reason for hiding this comment

omochi commented May 26, 2024 •

edited

Loading

`WebDriverHTTPClient` の公開

`WebDriverService` の実装

`withRetry` の公開

`WebDriver` モジュールへの名前変更

omochi May 27, 2024 •

edited

Loading