[インデックス 18698] ファイルの概要

このコミットは、Go言語の標準ライブラリ net/http パッケージ内の ConnState テストにおける不安定性（flakiness）を解消することを目的としています。具体的には、serve_test.go ファイル内のテストロジックが修正され、複数のコネクションの状態変化をより堅牢に追跡できるようになりました。

コミット

commit 1f0a1f4c8dd67ec1cb144bdca177dfed18eb3d6e
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Fri Feb 28 13:27:36 2014 -0800

    net/http: de-flake ConnState test
    
    LGTM=josharian
    R=golang-codereviews, josharian
    CC=golang-codereviews
    https://golang.org/cl/70270043

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/1f0a1f4c8dd67ec1cb144bdca177dfed18eb3d6e

元コミット内容

net/http: de-flake ConnState test

このコミットメッセージは簡潔ですが、その目的は明確です。net/http パッケージの ConnState 機能に関するテストが不安定（flaky）であり、その不安定性を解消するための変更であることを示しています。

変更の背景

ソフトウェア開発において、テストの「不安定性（flakiness）」は深刻な問題です。不安定なテストとは、コードの変更がないにもかかわらず、実行するたびに成功したり失敗したりするテストのことです。これは、テストが非決定的な要素（例: 並行処理のタイミング、外部リソースの可用性、システムクロックなど）に依存している場合に発生しがちです。

このコミットの背景には、net/http パッケージの ConnState 機能のテストが、このような不安定性を抱えていたという問題があります。ConnState は、HTTPサーバーがクライアントとのコネクションの状態変化（新規接続、アクティブ、アイドル、クローズなど）をフックするためのメカニズムを提供します。テストでは、複数のHTTPリクエストを処理する際に、これらのコネクション状態が期待通りに遷移するかどうかを検証していました。

しかし、複数のコネクションが並行して処理される環境では、それぞれのコネクションの状態変化イベントが発生する正確な順序は保証されません。元のテストでは、すべてのコネクションの状態変化を単一の線形リストに記録し、そのリストが期待される順序と一致するかどうかを検証していました。このアプローチでは、異なるコネクションからのイベントがインターリーブされる可能性があり、そのインターリーブの順序が実行ごとに異なるため、テストが不安定になっていました。

この不安定性は、CI/CDパイプラインの信頼性を低下させ、開発者が実際のバグとテストの不安定性による誤検出を区別するのに時間を費やす原因となります。そのため、このテストの不安定性を解消することは、開発効率とコードベースの信頼性を向上させる上で重要でした。

前提知識の解説

Go言語の net/http パッケージ

net/http パッケージは、Go言語でHTTPクライアントおよびサーバーを実装するための標準ライブラリです。このパッケージは、HTTPプロトコルの低レベルな詳細を抽象化し、開発者が簡単にWebアプリケーションを構築できるようにします。

http.Server.ConnState

http.Server 構造体には ConnState というフィールドがあります。これは、func(c net.Conn, cs ConnState) 型の関数ポインタです。この関数は、HTTPサーバーがクライアントとのTCPコネクションの状態が変化したときに呼び出されるコールバックメカニズムを提供します。

ConnState に渡される ConnState 型の列挙値は以下のいずれかです。

• http.StateNew: 新しいコネクションが確立され、HTTPリクエストの処理を待っている状態。
• http.StateActive: コネクションがリクエストを処理中（リクエストの読み込み中またはレスポンスの書き込み中）の状態。
• http.StateIdle: コネクションがアイドル状態であり、次のリクエストを待っている状態（Keep-Aliveコネクションの場合）。
• http.StateHijacked: コネクションがHTTPサーバーによってハイジャックされた状態。これは、WebSocketのようなプロトコルアップグレードや、カスタムプロトコルを実装する際に使用されます。コネクションがハイジャックされると、HTTPサーバーはそれ以降そのコネクションを管理しません。
• http.StateClosed: コネクションがクローズされた状態。

この ConnState コールバックは、コネクションプーリング、リソース管理、またはデバッグ目的でコネクションのライフサイクルを監視する際に非常に有用です。

テストの不安定性（Flakiness）

前述の通り、テストの不安定性とは、同じコードベースに対して同じテストを複数回実行したときに、結果が成功と失敗の間で変動する現象を指します。主な原因としては、以下のようなものがあります。

• 並行処理のタイミング依存性: 複数のゴルーチンが同時に実行される場合、それらの実行順序はOSのスケジューラやCPUの負荷によって変動します。テストが特定の実行順序に依存していると、その順序が崩れた場合に失敗します。
• 外部リソースの依存性: データベース、ネットワークサービス、ファイルシステムなどの外部リソースの状態がテスト実行ごとに異なる場合、テスト結果に影響を与える可能性があります。
• システムクロックの依存性: 時間に関連するロジックがテストに含まれる場合、テスト実行時のシステムクロックの精度や進捗によって結果が変わることがあります。
• 不適切なテストデータの管理: テスト間でデータが共有され、適切にクリーンアップされない場合、前のテストの残骸が次のテストに影響を与えることがあります。

不安定なテストは、開発者の信頼を損ない、CI/CDパイプラインのボトルネックとなるため、可能な限り排除されるべきです。

技術的詳細

このコミットの技術的な核心は、TestServerConnState テストにおけるコネクション状態の記録方法の変更にあります。

変更前の問題点

変更前は、stateLog というスライス（[]connIDAndState 型）を使用して、すべてのコネクションの状態変化イベントを発生順に記録していました。connIDAndState 構造体は、コネクションIDと対応する ConnState を保持していました。

type connIDAndState struct {
    connID int
    state  ConnState
}
var stateLog []connIDAndState

そして、ts.Config.ConnState コールバック内で、この stateLog にイベントを append していました。

stateLog = append(stateLog, connIDAndState{id, state})

テストの最後では、この stateLog が want という期待されるスライスと完全に一致するかどうかを比較していました。

want := []connIDAndState{
    {1, StateNew},
    {1, StateActive},
    // ...
    {2, StateNew},
    {2, StateActive},
    // ...
}

このアプローチの問題点は、複数のHTTPリクエスト（このテストでは3つのリクエストが並行して行われる）が異なるコネクションを使用する場合、それぞれのコネクションからの ConnState イベントが、OSのスケジューリングやネットワークのタイミングによって、stateLog に追加される順序が非決定論的になることです。例えば、コネクション1の StateIdle イベントの直後にコネクション2の StateNew イベントが来ることもあれば、その逆の順序になることもあります。しかし、want スライスは厳密な線形順序を期待しているため、実際のイベント順序が want と異なるとテストが失敗していました。これがテストの不安定性の原因でした。

変更後の解決策

このコミットでは、stateLog のデータ構造をスライスからマップ（map[int][]ConnState 型）に変更することで、この問題を解決しました。

var stateLog = map[int][]ConnState{}

このマップのキーはコネクションID（int）であり、値はそのコネクションIDに関連付けられた ConnState のスライス（[]ConnState）です。

ts.Config.ConnState コールバック内では、コネクションIDごとにそのコネクションの状態変化を記録するように変更されました。

stateLog[id] = append(stateLog[id], state)

これにより、各コネクションの状態変化は、そのコネクション内では順序が保証されたまま、マップの対応するスライスに記録されます。異なるコネクション間のイベントのインターリーブ順序はもはや重要ではありません。

テストの最後では、want も同様に map[int][]ConnState 型に変更され、コネクションIDごとの期待される状態遷移のシーケンスを定義するようになりました。

want := map[int][]ConnState{
    1: []ConnState{StateNew, StateActive, StateIdle, StateActive, StateClosed},
    2: []ConnState{StateNew, StateActive, StateIdle, StateActive, StateClosed},
    3: []ConnState{StateNew, StateActive, StateHijacked},
}

そして、テストは stateLog マップと want マップを比較します。この比較では、各コネクションIDについて、そのコネクションの状態遷移のシーケンスが期待通りであるかどうかが検証されます。コネクション間のイベントの相対的な順序は無視されるため、テストは並行処理のタイミングに依存しなくなり、安定しました。

この変更は、並行処理を含むテストにおいて、イベントのグローバルな順序ではなく、エンティティごとのローカルな順序のみが重要である場合に適用される一般的なパターンです。

コアとなるコードの変更箇所

src/pkg/net/http/serve_test.go ファイルにおいて、以下の主要な変更が行われました。

1. connIDAndState 構造体の削除:

   -type connIDAndState struct {
   -	connID int
   -	state  ConnState
   -}
   

2. stateLog 変数の型変更: スライスからマップへ変更。

   -	var stateLog []connIDAndState
   +	var stateLog = map[int][]ConnState{}
   

3. ConnState コールバック内の stateLog への記録方法の変更: コネクションIDごとのスライスに状態を追加するように変更。

   -		stateLog = append(stateLog, connIDAndState{id, state})
   +		stateLog[id] = append(stateLog[id], state)
   

4. want 変数の型と内容の変更: 期待される状態もマップ形式に変更。

   -	want := []connIDAndState{
   -		{1, StateNew},
   -		{1, StateActive},
   -		{1, StateIdle},
   -		{1, StateActive},
   -		{1, StateClosed},
   -
   -		{2, StateNew},
   -		{2, StateActive},
   -		{2, StateIdle},
   -		{2, StateActive},
   -		{2, StateClosed},
   -
   -		{3, StateNew},
   -		{3, StateActive},
   -		{3, StateHijacked},
   +	want := map[int][]ConnState{
   +		1: []ConnState{StateNew, StateActive, StateIdle, StateActive, StateClosed},
   +		2: []ConnState{StateNew, StateActive, StateIdle, StateActive, StateClosed},
   +		3: []ConnState{StateNew, StateActive, StateHijacked},
   	}
   

5. logString ヘルパー関数の変更: マップ形式の stateLog を整形して出力するように変更。

   -	logString := func(l []connIDAndState) string {
   +	logString := func(m map[int][]ConnState) string {
    		var b bytes.Buffer
   -		for _, cs := range l {
   -			fmt.Fprintf(&b, "[%d %s] ", cs.connID, cs.state)
   +		for id, l := range m {
   +			fmt.Fprintf(&b, "Conn %d: ", id)
   +			for _, s := range l {
   +				fmt.Fprintf(&b, "%s ", s)
   +			}
    		}
    		return b.String()
    	}
   

コアとなるコードの解説

このコミットの核心は、テストの検証ロジックが、複数の並行するコネクションからのイベントのグローバルな順序に依存するのではなく、各コネクションごとのイベントの順序にのみ依存するように変更された点です。

変更前は、stateLog が単一のスライスであったため、コネクション1のイベント、コネクション2のイベント、コネクション3のイベントが、発生した順にすべてこのスライスに追加されていました。しかし、並行処理の性質上、これらのイベントがスライスに追加される正確な順序は、テスト実行ごとに変動する可能性がありました。例えば、コネクション1の StateIdle の後にコネクション2の StateNew が来ることもあれば、その逆もあり得ます。元のテストは、このグローバルな順序が want スライスと完全に一致することを期待していたため、非決定的な順序の変動によってテストが失敗していました。

変更後は、stateLog が map[int][]ConnState となりました。これにより、各コネクション（connID で識別される）は、自身の状態遷移のシーケンスを独立したスライスとしてマップ内に保持します。

• stateLog[1] にはコネクション1の [StateNew, StateActive, StateIdle, StateActive, StateClosed] が格納されます。
• stateLog[2] にはコネクション2の [StateNew, StateActive, StateIdle, StateActive, StateClosed] が格納されます。
• stateLog[3] にはコネクション3の [StateNew, StateActive, StateHijacked] が格納されます。

テストの検証時には、このマップの各エントリ（つまり、各コネクションごとの状態遷移シーケンス）が、want マップの対応するエントリと一致するかどうかを個別に確認します。コネクション1のシーケンスが期待通りか、コネクション2のシーケンスが期待通りか、といった具合です。

このアプローチにより、異なるコネクションからのイベントが stateLog マップに記録される際の相対的な順序はもはやテスト結果に影響を与えません。重要なのは、各コネクションが自身のライフサイクル内で正しい状態遷移をたどったかどうかだけです。これにより、テストは並行処理の非決定性に強くなり、安定して動作するようになりました。

関連リンク

• Go言語の net/http パッケージのドキュメント: https://pkg.go.dev/net/http
• Go言語の http.ConnState のドキュメント: https://pkg.go.dev/net/http#ConnState
• Go言語の http.Server.ConnState のドキュメント: https://pkg.go.dev/net/http#Server.ConnState

参考にした情報源リンク

• Go言語の公式リポジトリ: https://github.com/golang/go
• Go言語のコードレビューシステム (Gerrit): https://go-review.googlesource.com/
• このコミットのGerritチェンジリスト: https://golang.org/cl/70270043 (現在はGitHubにリダイレクトされます)
• テストの不安定性に関する一般的な情報源 (例: Martin Fowler の "Flaky Tests"): https://martinfowler.com/articles/flakyTests.html