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

このコミットは、Go言語の標準ライブラリ net/http パッケージ内のテストコード transport_test.go に対する修正です。具体的には、テスト関数内で起動される非テスト関数（ゴルーチン）において、エラー報告の方法を testing.Fatalf から testing.Errorf へと変更し、さらに TestTransportMaxPerHostIdleConns テストにおけるnilポインタ参照の可能性を回避する修正が含まれています。

コミット

net/http: 非テスト関数ゴルーチンにおいて testing.Fatalf の代わりに testing.Errorf を使用するように変更。

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

https://github.com/golang/go/commit/d7039b71a92bb45909c93970bb5593414cb2cdbe

元コミット内容

net/http: make use of testing.Errorf instead of testing.Fatalf in non-test function goroutines

See testing.FailNow for further information.
Also avoid nil pointer derefernce in TestTransportMaxPerHostIdleConns.

LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://golang.org/cl/76470043

変更の背景

このコミットには主に2つの背景があります。

テストゴルーチン内での testing.Fatalf の不適切な使用: Goの testing パッケージにおいて、Fatalf はテストを即座に終了させる（runtime.Goexit を呼び出す）機能を持っています。しかし、これはテスト関数自体（func TestXxx(t *testing.T)）のゴルーチン内で呼び出されることを想定しています。テスト関数から派生した別のゴルーチン（例えば go func() { ... }() で起動されるもの）内で Fatalf を呼び出すと、そのゴルーチンのみが終了し、メインのテストゴルーチンは継続して実行されます。これにより、テストが期待通りに失敗せず、テストスイート全体がハングアップしたり、誤った結果を報告したりする可能性があります。このコミットは、このような状況で Fatalf が使用されていた箇所を Errorf に変更することで、テストの健全性を向上させています。Errorf はエラーを報告するだけで、ゴルーチンを終了させません。これにより、エラーが発生してもテストゴルーチンは最後まで実行され、他のエラーも検出できるようになります。
TestTransportMaxPerHostIdleConns におけるnilポインタ参照の可能性: TestTransportMaxPerHostIdleConns テスト内で、ioutil.ReadAll(resp.Body) の結果を処理する際に、resp.Body がnilである可能性が考慮されていませんでした。resp がエラーでnilになる場合、その後の resp.Body へのアクセスがnilポインタ参照を引き起こし、パニックを発生させる可能性がありました。このコミットは、resp, err := c.Get(ts.URL) の直後にエラーチェックを追加し、エラーが発生した場合は早期にリターンすることで、この潜在的なnilポインタ参照の問題を解決しています。

前提知識の解説

Go言語の `testing` パッケージ

Go言語には、ユニットテストやベンチマークテストを記述するための組み込みの testing パッケージがあります。

*testing.T: テスト関数に渡される構造体で、テストの状態管理、エラー報告、テストのスキップなどを行います。
t.Error(args ...interface{}) / t.Errorf(format string, args ...interface{}): テスト中にエラーが発生したことを報告しますが、テストの実行は継続します。テストは最終的に失敗としてマークされます。
t.Fatal(args ...interface{}) / t.Fatalf(format string, args ...interface{}): テスト中に致命的なエラーが発生したことを報告し、現在のテスト関数を即座に終了させます。これは runtime.Goexit を呼び出すことで実現されます。runtime.Goexit は現在のゴルーチンを終了させますが、プログラム全体は終了させません。
testing.FailNow(): Fatalf や Fatal が内部的に呼び出す関数で、現在のテストゴルーチンを終了させます。

ゴルーチン (Goroutines)

Go言語のゴルーチンは、軽量な並行実行の単位です。関数呼び出しの前に go キーワードを付けることで、新しいゴルーチンとしてその関数を実行できます。ゴルーチンはOSのスレッドよりもはるかに軽量であり、数千、数万のゴルーチンを同時に実行することが可能です。

`net/http` パッケージ

Go言語の標準ライブラリの一部であり、HTTPクライアントとサーバーの実装を提供します。このコミットで修正された transport_test.go は、HTTPクライアントのトランスポート層（ネットワーク通信の具体的な処理を行う部分）のテストコードです。

`ioutil.ReadAll`

io/ioutil パッケージ（Go 1.16以降は io パッケージに統合）の関数で、io.Reader からすべてのデータを読み込み、バイトスライスとして返します。HTTPレスポンスボディを読み込む際によく使用されます。

nilポインタ参照 (Nil Pointer Dereference)

Go言語では、ポインタが nil （何も指していない状態）であるにもかかわらず、そのポインタが指す値にアクセスしようとすると、ランタイムパニック（実行時エラー）が発生します。これは一般的なプログラミングエラーの一つです。

技術的詳細

このコミットの主要な技術的変更点は、testing.Fatalf の使用箇所を testing.Errorf に変更し、それに伴い早期リターンを追加したことです。

`testing.Fatalf` と `testing.Errorf` の使い分け

testing.Fatalf: テスト関数（func TestXxx(t *testing.T)）のメインゴルーチン内で、そのテストがこれ以上続行できないほど致命的なエラーが発生した場合に使用します。Fatalf が呼び出されると、そのテスト関数はそこで実行を停止し、テストは失敗としてマークされます。
testing.Errorf: テスト中にエラーが発生したが、テストの残りの部分も実行して他のエラーを検出したい場合に使用します。Errorf はエラーを報告するだけで、テストの実行は継続します。テストは最終的に失敗としてマークされます。

問題は、go func() { ... }() のように新しいゴルーチンを起動し、その中で Fatalf を呼び出す場合です。Fatalf は呼び出されたゴルーチンを終了させますが、テスト関数を呼び出したメインのゴルーチンは終了させません。これにより、メインのテストゴルーチンは、エラーが発生したにもかかわらず、その後の処理を続行しようとする可能性があります。これは、テストのロジックが期待する状態と異なる状態で実行されることになり、テストの信頼性を損ないます。

このコミットでは、非テスト関数ゴルーチン内で Fatalf が使用されていた箇所を Errorf に変更し、その後に return を追加しています。Errorf はゴルーチンを終了させないため、return を明示的に追加することで、エラー発生後の不要な処理を防ぎ、かつメインのテストゴルーチンが継続して実行されることを保証しています。これにより、テストスイート全体がハングアップすることなく、エラーが適切に報告されるようになります。

nilポインタ参照の回避

TestTransportMaxPerHostIdleConns テストでは、HTTPリクエストの c.Get(ts.URL) がエラーを返す可能性があります。元のコードでは、このエラーチェックが不十分でした。

		resp, err := c.Get(ts.URL)
		if err != nil {
			t.Error(err)
		}
		_, err = ioutil.ReadAll(resp.Body) // respがnilの場合、ここでパニック
		if err != nil {
			t.Fatalf("ReadAll: %v", err)
		}

c.Get(ts.URL) がエラーを返した場合、resp は nil になる可能性があります。その状態で resp.Body にアクセスしようとすると、nilポインタ参照によるパニックが発生します。

修正後のコードでは、c.Get(ts.URL) のエラーチェックの直後に return を追加しています。

		resp, err := c.Get(ts.URL)
		if err != nil {
			t.Error(err)
			return // エラーが発生したらここでゴルーチンを終了
		}
		// respがnilでないことが保証される
		if _, err := ioutil.ReadAll(resp.Body); err != nil {
			t.Errorf("ReadAll: %v", err)
			return
		}

これにより、c.Get がエラーを返した場合、そのゴルーチンは即座に終了し、resp.Body へのアクセスは行われません。これにより、nilポインタ参照によるパニックが回避されます。

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

src/pkg/net/http/transport_test.go ファイルが変更されています。

--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -349,10 +349,11 @@ func TestTransportMaxPerHostIdleConns(t *testing.T) {
 		resp, err := c.Get(ts.URL)
 		if err != nil {
 			t.Error(err)
+			return
 		}
-		_, err = ioutil.ReadAll(resp.Body)
-		if err != nil {
-			t.Fatalf("ReadAll: %v", err)
+		if _, err := ioutil.ReadAll(resp.Body); err != nil {
+			t.Errorf("ReadAll: %v", err)
+			return
 		}
 		donech <- true
 	}
@@ -1808,15 +1809,18 @@ func TestTransportTLSHandshakeTimeout(t *testing.T) {
 		cl := &Client{Transport: tr}
 		_, err := cl.Get("https://dummy.tld/")
 		if err == nil {
-			t.Fatal("expected error")
+			t.Error("expected error")
+			return
 		}
 		ue, ok := err.(*url.Error)
 		if !ok {
-			t.Fatalf("expected url.Error; got %#v", err)
+			t.Errorf("expected url.Error; got %#v", err)
+			return
 		}
 		ne, ok := ue.Err.(net.Error)
 		if !ok {
-			t.Fatalf("expected net.Error; got %#v", err)
+			t.Errorf("expected net.Error; got %#v", err)
+			return
 		}
 		if !ne.Timeout() {
 			t.Error("expected timeout error; got %v", err)

コアとなるコードの解説

`TestTransportMaxPerHostIdleConns` テスト内の変更

変更前:

		resp, err := c.Get(ts.URL)
		if err != nil {
			t.Error(err)
		}
		_, err = ioutil.ReadAll(resp.Body)
		if err != nil {
			t.Fatalf("ReadAll: %v", err)
		}

変更後:
```
		resp, err := c.Get(ts.URL)
		if err != nil {
			t.Error(err)
			return // エラー発生時に早期リターン
		}
		if _, err := ioutil.ReadAll(resp.Body); err != nil { // エラーチェックと同時にReadAllを呼び出し
			t.Errorf("ReadAll: %v", err) // FatalfからErrorfに変更
			return // エラー発生時に早期リターン
		}
```
c.Get(ts.URL) の呼び出しでエラーが発生した場合、resp は nil になる可能性があります。変更前は t.Error(err) でエラーを報告するだけで、その後の ioutil.ReadAll(resp.Body) が nil ポインタ参照を引き起こす可能性がありました。変更後は、t.Error(err) の後に return を追加することで、エラーが発生した場合はそれ以上処理を進めないようにしています。また、ioutil.ReadAll のエラー報告も t.Fatalf から t.Errorf に変更され、同様に return が追加されています。これは、この部分がテスト関数から起動されたゴルーチン内で実行されるため、Fatalf を使用するとメインのテストゴルーチンがハングアップする可能性があるためです。

`TestTransportTLSHandshakeTimeout` テスト内の変更