Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の標準ライブラリ image/png パッケージ内のテストコードにおけるバグ修正です。具体的には、PNG画像の比較テスト（ゴールデンファイルテスト）において、bufio.Scanner.Scan メソッドの戻り値の解釈を誤っていたために、テストが早期に終了してしまう問題を修正しています。

コミット

commit eb788045d85f48980b8513a71661fcf3100c98c8
Author: Nigel Tao <nigeltao@golang.org>
Date:   Fri Jun 13 17:43:02 2014 +1000

    image/png: fix compare-to-golden-file test.
    
    bufio.Scanner.Scan returns whether the scan succeeded, not whether it
    is done, so the test was mistakenly breaking early.
    
    LGTM=r
    R=r
    CC=golang-codereviews
    https://golang.org/cl/93670045
---
 src/pkg/image/png/reader_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pkg/image/png/reader_test.go b/src/pkg/image/png/reader_test.go
index ac0d949a9d..0bc3c8d4a1 100644
--- a/src/pkg/image/png/reader_test.go
+++ b/src/pkg/image/png/reader_test.go
@@ -235,8 +235,8 @@ func TestReader(t *testing.T) {

 		// Compare the two, in SNG format, line by line.
 		for {
-			pdone := pb.Scan()
-			sdone := sb.Scan()
+			pdone := !pb.Scan()
+			sdone := !sb.Scan()
 			if pdone && sdone {
 				break
 			}

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

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

元コミット内容

image/png: fix compare-to-golden-file test.

bufio.Scanner.Scan returns whether the scan succeeded, not whether it
is done, so the test was mistakenly breaking early.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/93670045

変更の背景

このコミットは、image/png パッケージのテストスイートにおける既存のバグを修正するために行われました。具体的には、PNG画像のエンコード/デコード結果を既知の正しい出力（ゴールデンファイル）と比較するテストにおいて、比較処理が意図せず途中で終了してしまう問題がありました。

問題の原因は、bufio.Scanner の Scan() メソッドの戻り値の解釈の誤りにありました。開発者は Scan() が「スキャンが完了したかどうか」を示すと考えていましたが、実際には「次のトークン（この場合は行）が正常に読み取れたかどうか」を示すものでした。この誤解により、両方のスキャナーが正常に次の行を読み取れた場合にループを終了するという誤ったロジックが組まれており、結果としてファイル全体が比較される前にテストが終了していました。

この修正は、テストの信頼性を確保し、image/png パッケージが正しく機能していることを保証するために不可欠でした。

前提知識の解説

1. ゴールデンファイルテスト (Golden File Testing)

ゴールデンファイルテストは、ソフトウェアの出力が既知の正しい出力（「ゴールデンファイル」または「リファレンスファイル」と呼ばれる）と一致することを検証するテスト手法です。特に画像処理、コード生成、ドキュメント生成など、複雑な出力を持つシステムで有効です。 テストの実行時に生成された出力と、事前に作成されたゴールデンファイルを比較し、両者が完全に一致すればテストは成功とみなされます。これにより、意図しない変更や回帰バグを検出できます。

2. bufio.Scanner

Go言語の bufio パッケージは、バッファリングされたI/O操作を提供します。bufio.Scanner は、io.Reader からデータを読み込み、それをトークン（行、単語、カスタム区切り文字など）に分割するためのユーティリティです。

  • bufio.NewScanner(r io.Reader): 新しい Scanner を作成します。
  • scanner.Scan() bool: このメソッドが bufio.Scanner の核心です。
    • 入力から次のトークンを読み込もうと試みます。
    • 成功した場合（次のトークンが利用可能になった場合）は true を返します。
    • 入力の終端に達した場合（EOF）や、読み込み中にエラーが発生した場合は false を返します。
    • Scan() が true を返した後、scanner.Text() や scanner.Bytes() を呼び出すことで、読み込まれたトークンを取得できます。
  • scanner.Text() string: 現在のトークンを文字列として返します。
  • scanner.Err() error: Scan() が false を返した後、スキャンが停止した原因となったエラーを返します。EOFの場合は nil を返します。

一般的な bufio.Scanner の使用パターンは以下のようになります。

scanner := bufio.NewScanner(someReader)
for scanner.Scan() { // Scan()がtrueを返す限りループを続ける
    line := scanner.Text()
    // 行を処理
}
if err := scanner.Err(); err != nil {
    // エラー処理
}

このパターンからわかるように、Scan() が true を返すことは「まだ読み込むべきデータがある」ことを意味し、false を返すことが「読み込みが終了した（またはエラーが発生した）」ことを意味します。

技術的詳細

このコミットの技術的詳細は、bufio.Scanner.Scan() メソッドの戻り値のセマンティクスに関する誤解に集約されます。

元のコードでは、2つの bufio.Scanner インスタンス (pb と sb) を使用して、それぞれ生成されたPNGデータとゴールデンファイルデータをSNG（Simple Network Graphics）形式で一行ずつ比較していました。ループの終了条件は以下のようになっていました。

for {
    pdone := pb.Scan()
    sdone := sb.Scan()
    if pdone && sdone {
        break
    }
    // ... 比較ロジック ...
}

このロジックでは、pdone と sdone が両方とも true になった場合にループを break していました。しかし、bufio.Scanner.Scan() は、次の行が正常に読み取れた場合に true を返します。したがって、pdone && sdone が true になるのは、両方のファイルにまだ読み取るべき行が存在する場合です。この条件で break してしまうと、両方のファイルがまだ読み取り途中であるにもかかわらず、比較ループが終了してしまいます。結果として、ファイル全体が比較されず、テストが不正確になっていました。

修正は非常にシンプルで、Scan() の戻り値を論理的に反転させることで、正しいループ終了条件を実現しています。

for {
    pdone := !pb.Scan() // Scan()がfalse（読み込み終了またはエラー）の場合にtrueになる
    sdone := !sb.Scan() // Scan()がfalse（読み込み終了またはエラー）の場合にtrueになる
    if pdone && sdone {
        break
    }
    // ... 比較ロジック ...
}

この変更により、pdone と sdone は、それぞれのスキャナーがデータの終端に達したか、またはエラーが発生した場合に true となります。したがって、pdone && sdone が true になるのは、両方のスキャナーが完全に読み取りを終えた（または両方でエラーが発生した）場合のみとなり、テストは期待通りにファイル全体を比較できるようになりました。

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

変更は src/pkg/image/png/reader_test.go ファイルの TestReader 関数内で行われています。

--- a/src/pkg/image/png/reader_test.go
+++ b/src/pkg/image/png/reader_test.go
@@ -235,8 +235,8 @@ func TestReader(t *testing.T) {

 		// Compare the two, in SNG format, line by line.
 		for {
-			pdone := pb.Scan()
-			sdone := sb.Scan()
+			pdone := !pb.Scan()
+			sdone := !sb.Scan()
 			if pdone && sdone {
 				break
 			}

具体的には、pdone と sdone の変数への代入行が変更されています。

コアとなるコードの解説

変更された行は以下の通りです。

変更前:

pdone := pb.Scan()
sdone := sb.Scan()

このコードでは、pb.Scan() と sb.Scan() の戻り値（bool 型）がそのまま pdone と sdone に代入されていました。前述の通り、Scan() は次のトークンが正常に読み取れた場合に true を返します。したがって、pdone と sdone が true の場合、それは「まだ読み取るべきデータがある」ことを意味していました。

変更後:

pdone := !pb.Scan()
sdone := !sb.Scan()

この変更では、pb.Scan() と sb.Scan() の戻り値に論理否定演算子 ! が適用されています。これにより、

  • pb.Scan() が true（次のトークンが読み取れた）の場合、!pb.Scan() は false になります。
  • pb.Scan() が false（読み込み終了またはエラー）の場合、!pb.Scan() は true になります。

したがって、変更後の pdone と sdone は、「対応するスキャナーが読み取りを完了した（またはエラーで停止した）」場合に true となります。

そして、ループの終了条件 if pdone && sdone { break } は、両方のスキャナーが読み取りを完了した場合にのみ true となり、ループが終了するようになります。これにより、両方のファイルが完全に比較されるまでループが継続されるという、テストの本来の意図が正しく実装されました。

この修正は、Go言語の標準ライブラリのテストの堅牢性を高め、image/png パッケージの品質保証に貢献しています。

関連リンク

参考にした情報源リンク