Verify if tests communicate with just spawned dev server #479

omochi · 2024-06-05T13:16:16Z

背景・課題

DevCommandTests と FrontendDevServerTests では、テストケースの中で dev server を起動して、
それと通信して挙動を検証しています。

しかし、テスト実行中に別のターミナルで dev server が起動していたり、
全く別のプロセスがポートを使用していたり、
開発中のバグの影響で、以前起動したテスト用のプロセスがゾンビ化して生き残っていたりすると、
テストしようとした dev server が待ち受けポートを利用できずに起動に失敗してしまいます。

これは実際に、SIGTERM を SIGINT に意図せず変換してしまうバグを修正した時に、
SIGTERM のハンドルがされずにゾンビプロセスが生まれることによって、
CIがわかりづらい壊れ方をするなどの現象を引き起こしました。

起動に失敗したことによりテストが停止すれば良いのですが、
この起動の失敗を検出するのは困難です。
carton dev コマンドは swift ツールチェーンのインストールなども行うため、
プロセスが正常に実行されているのか、
将来的にサーバの起動に失敗する状況にあるかわからないからです。

起動時間が読めないこともあって、テストケースはHTTPリクエストをしばらくリトライします。
そして、別の理由で先に存在していた dev server がいた場合は、
テストプロセスのHTTP通信が成功してしまうため、
テストが誤って成功したり、予期し得ないレスポンスが返って意味不明な失敗をしたりします。

また、ゾンビプロセスが残っていることがわかった時、
それを特定して kill するまでの手順も多少面倒です。
macOS は $ lsof -i -P の実行に何故か結構待ち時間があります。

提案

テストを安定させるために、
HTTPレスポンスを返してきたサーバが期待したプロセスかどうか検証できるようにします。

そのため、HTTPレスポンスヘッダーの Server: フィールドを利用して、

Server: carton dev server/1.0.4 (PID 12345)

のような情報を返すようにします。

テストケースではこれを検証することによって、
そもそもそのテストケースのために起動したサーバプロセスと通信しているのかを検証し、
そうでなければエラーを出して停止するようにします。

また、これは開発中にゾンビプロセスが生じた場合にも便利です。
$ curl -i localhost:8080 でヘッダを見れば pid がわかるからです。

pid の指定

frontend を起動した場合はそれが直接サーバプロセスになりますが、
driver を起動した場合は、
carton driver → swiftpm plugin runner → carton plugin → carton frontend
と4階層のプロセスツリーができます。

この時、テストケースが知っているのは自分が起動した手元のdriverのpidなので、
サーバはその driver の pid を通知して欲しいです。

そこで、carton plugin と carton frontend に --pid オプションを追加して、
dev server が広告する carton のプロセス番号を指定できるようにします。

結果

以下のようにテストがうまく停止するようになりました。

FrontendDevServerTestsで検出成功した例

[omochi@omochi-mbp carton (server-name *)]$ swift test --filter FrontendDevServerTests
Building for debugging...
[5/5] Write swift-version-3874884F1997D323.txt
Build complete! (0.13s)
Test Suite 'Selected tests' started at 2024-06-05 22:12:14.472.
Test Suite 'cartonPackageTests.xctest' started at 2024-06-05 22:12:14.473.
Test Suite 'FrontendDevServerTests' started at 2024-06-05 22:12:14.473.
Test Case '-[CartonCommandTests.FrontendDevServerTests testDevServerPublish]' started.
Running...
swift build --target carton-frontend
Building for debugging...
[0/1] Write swift-version-3874884F1997D323.txt
Build complete! (0.18s)
`swift` process finished successfully
warning: 'devservertestapp': 'app' was identified as an executable target given the presence of a 'main.swift' file. Starting with tools version 5.4.0 executable targets should be declared as 'executableTarget()'
Building for debugging...
[0/3] Write swift-version-3874884F1997D323.txt
Build complete! (0.11s)
Error: bind(descriptor:p
tr:bytes:): Address already in use (errn
o: 48)
/Users/omochi/github/swiftwasm/carton/Tests/CartonCommandTests/CommandTestHelper.swift:152: error: -[CartonCommandTests.FrontendDevServerTests testDevServerPublish] : failed: caught error: "Expected PID 51718 but got PID 51474."
Test Case '-[CartonCommandTests.FrontendDevServerTests testDevServerPublish]' failed (3.562 seconds).
Test Suite 'FrontendDevServerTests' failed at 2024-06-05 22:12:18.035.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.562 (3.562) seconds
Test Suite 'cartonPackageTests.xctest' failed at 2024-06-05 22:12:18.035.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.562 (3.562) seconds
Test Suite 'Selected tests' failed at 2024-06-05 22:12:18.035.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.562 (3.563) seconds

DevCommandTestsで検出成功した例

[omochi@omochi-mbp carton (server-name =)]$ swift test --filter DevCommandTests.testWithArguments
Building for debugging...
[5/5] Write swift-version-3874884F1997D323.txt
Build complete! (0.13s)
Test Suite 'Selected tests' started at 2024-06-05 22:14:29.061.
Test Suite 'cartonPackageTests.xctest' started at 2024-06-05 22:14:29.062.
Test Suite 'DevCommandTests' started at 2024-06-05 22:14:29.062.
Test Case '-[CartonCommandTests.DevCommandTests testWithArguments]' started.
Building for debugging...
[0/3] Write swift-version-3874884F1997D323.txt
Build complete! (0.11s)
- checking Swift compiler path: /Users/omochi/.carton/sdk/wasm-5.9.2-RELEASE/usr/bin/swift
- checking Swift compiler path: /Users/omochi/.swiftenv/versions/wasm-5.9.2-RELEASE/usr/bin/swift
- checking Swift compiler path: /Users/omochi/Library/Developer/Toolchains/swift-wasm-5.9.2-RELEASE.xctoolchain/usr/bin/swift
Inferring basic settings...
- swift executable:
/Users/omochi/Library/Developer/Toolchains/swift-wasm-5.9.2-RELEASE.xctoolchain/usr/bin/swift
SwiftWasm Swift version 5.9.2 (swift-5.9.2-RELEASE)
Target: x86_64-apple-darwin23.5.0
Building for debugging...
Build complete! (0.71s)
error: Plugin ended with exit code 1
Building "my-echo"

Watching these directories for changes:
/Users/omochi/github/swiftwasm/carton/Tests/Fixtures/EchoExecutable/Sources/my-echo

Error: bind(descriptor:ptr:bytes:): Address already in use (errno: 48)
Running "/Users/omochi/Library/Developer/Toolchains/swift-wasm-5.9.2-RELEASE.xctoolchain/usr/bin/swift" "package" "--triple" "wasm32-unknown-wasi" "--scratch-path" "/Users/omochi/github/swiftwasm/carton/Tests/Fixtures/EchoExecutable/.build/carton" "--disable-sandbox" "plugin" "carton-dev" "--verbose" "--port" "8080" "--skip-auto-open" "--pid" "53193"
/Users/omochi/github/swiftwasm/carton/Tests/CartonCommandTests/CommandTestHelper.swift:152: error: -[CartonCommandTests.DevCommandTests testWithArguments] : failed: caught error: "Expected PID 53193 but got PID 51474."
Test Case '-[CartonCommandTests.DevCommandTests testWithArguments]' failed (3.085 seconds).
Test Suite 'DevCommandTests' failed at 2024-06-05 22:14:32.147.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.085 (3.085) seconds
Test Suite 'cartonPackageTests.xctest' failed at 2024-06-05 22:14:32.147.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.085 (3.085) seconds
Test Suite 'Selected tests' failed at 2024-06-05 22:14:32.147.
	 Executed 1 test, with 1 failure (1 unexpected) in 3.085 (3.086) seconds

omochi

self comment

omochi · 2024-06-06T13:28:18Z

Plugins/CartonDevPlugin/CartonDevPluginCommand.swift

-        + (options.verbose ? ["--verbose"] : [])
-        + extractor.remainingArguments
-    )
+    var args: [String] = [


ここ、巨大なリテラルと演算子が混ざっていて型推論が壊れやすいです。
正しいコードを書いていればいいですが、
作業中に間違ったコードを書いたりした時に IDEサポートがなくなってしまうのでやりづらいです。
式を分割して安定させます。

omochi · 2024-06-06T13:28:47Z

Plugins/CartonDevPlugin/CartonDevPluginCommand.swift

+      "--build-request", buildRequestPipe,
+      "--build-response", buildResponsePipe
+    ]
+    args += (options.pid.map { ["--pid", $0] } ?? [])


パッチとしての挙動の変更は --pid オプションを扱うこの部分です。

omochi · 2024-06-06T13:29:07Z

Plugins/CartonTestPlugin/CartonTestPluginCommand.swift

+      "--environment", options.environment.rawValue,
+      "--plugin-work-directory", context.pluginWorkDirectory.string
+    ]
+    args += (options.pid.map { ["--pid", $0] } ?? [])


ここで --pid の転送を追加しました。
また、式を分割しています。

omochi · 2024-06-06T13:29:38Z

Sources/CartonDriver/CartonDriverCommand.swift

@@ -64,6 +66,7 @@ func derivePackageCommandArguments(
    cartonPluginArguments = ["--output", "Bundle"] + cartonPluginArguments
  case "dev":
    packageArguments += ["--disable-sandbox"]
+    cartonPluginArguments += ["--pid", pid.description]


driverがpluginを起動するときに、
devとtestだったらdriverのpidを渡します。

omochi · 2024-06-06T13:30:13Z

Sources/CartonKit/Server/Server.swift

+
+  public static let serverName = "carton dev server"
+
+  public struct ServerNameField: CustomStringConvertible {


Server: ヘッダーフィールドの型です。
テスト側で受け取って読み取りたいのでパーサ付き。

omochi · 2024-06-06T13:32:51Z

Tests/CartonCommandTests/DevCommandTests.swift

@@ -82,6 +82,7 @@ final class DevCommandTests: XCTestCase {

    let (response, data) = try await fetchDevServerWithRetry(at: try URL(string: url).unwrap("url"))
    XCTAssertEqual(response.statusCode, 200, "Response was not ok")
+    try checkServerNameField(response: response, expectedPID: process.process.processID)


通信相手はちゃんとテスト対象のプロセスだったのか検証します。

omochi · 2024-06-06T13:34:00Z

Tests/CartonCommandTests/FrontendDevServerTests.swift

@@ -5,6 +5,70 @@ import CartonKit
 import SwiftToolchain
 import WebDriver

+struct DevServerClient {


元々ここに書いてあったロジックをまとめました。
主な目的は fetchString などの関数に暗黙のパラメータとして起動したプロセスを参照することです。

これによりテストコードのシンプルさを保ったまま、
フェッチ関数の内部で通信相手を検証できます。

omochi · 2024-06-06T13:34:54Z

Tests/CartonCommandTests/FrontendDevServerTests.swift

+    at url: URL,
+    file: StaticString = #file, line: UInt = #line
+  ) async throws -> Data {
+    let (response, body) = try await withRetry(


これ元々は外側でretryしていましたが、
これはサーバの起動待ちが目的であって、
通信相手の検証処理などはretryしてもしょうがないため、
通信処理だけをリトライするようにここに移しました。

omochi · 2024-06-06T13:35:37Z

Tests/CartonCommandTests/FrontendDevServerTests.swift

-        }, stderr: { (_) in },
-        redirectStderr: true
-      )
+    let cl = try DevServerClient(


まとめたものを使っています。
見かけがスッキリしつつ、このテストケースのロジックは変わっていません。

omochi · 2024-06-06T13:36:03Z

Tests/Fixtures/EchoExecutable/Package.swift

@@ -5,5 +5,5 @@ let package = Package(
  name: "Foo",
  products: [.executable(name: "my-echo", targets: ["my-echo"])],
  dependencies: [.package(path: "../../..")],
-  targets: [.target(name: "my-echo")]
+  targets: [.executableTarget(name: "my-echo")]


.target は古い書き方のため警告が出るので新しい書き方にしました。

omochi · 2024-06-06T13:36:44Z

@kateinoigakukun こちら確認をお願いします。

kateinoigakukun · 2024-06-06T13:46:38Z

Sources/CartonFrontend/Commands/CartonFrontendDevCommand.swift

@@ -113,6 +113,8 @@ struct CartonFrontendDevCommand: AsyncParsableCommand {
  )
  var mainWasmPath: String

+  @Option(name: .long) var pid: Int32?


Could you hide the internal option from the help by help: .hidden?

そういうのがあるんですね。対応しました。

kateinoigakukun

👍

omochi added 2 commits June 5, 2024 20:58

publish Server header

c035d77

check pid in Server field

90d1b0e

omochi commented Jun 6, 2024

View reviewed changes

omochi marked this pull request as ready for review June 6, 2024 13:36

kateinoigakukun reviewed Jun 6, 2024

View reviewed changes

hide pid option

976d467

omochi requested a review from kateinoigakukun June 6, 2024 14:26

kateinoigakukun approved these changes Jun 6, 2024

View reviewed changes

kateinoigakukun changed the title ~~自動テストにおいてテスト対象のdev serverと通信しているか検証する~~ Verify if tests communicates with just spawned dev server Jun 6, 2024

kateinoigakukun changed the title ~~Verify if tests communicates with just spawned dev server~~ Verify if tests communicate with just spawned dev server Jun 6, 2024

kateinoigakukun enabled auto-merge (squash) June 6, 2024 14:40

kateinoigakukun merged commit f734c39 into swiftwasm:main Jun 6, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify if tests communicate with just spawned dev server #479

Verify if tests communicate with just spawned dev server #479

omochi commented Jun 5, 2024 •

edited

Loading

omochi left a comment

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi Jun 6, 2024

omochi commented Jun 6, 2024

kateinoigakukun Jun 6, 2024

omochi Jun 6, 2024

kateinoigakukun left a comment


		public static let serverName = "carton dev server"

		public struct ServerNameField: CustomStringConvertible {

Verify if tests communicate with just spawned dev server #479

Verify if tests communicate with just spawned dev server #479

Conversation

omochi commented Jun 5, 2024 • edited Loading

背景・課題

提案

pid の指定

結果

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omochi commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kateinoigakukun left a comment

Choose a reason for hiding this comment

omochi commented Jun 5, 2024 •

edited

Loading