40
5

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

HRBrainAdvent Calendar 2022

Day 7

Goのslice比較でテストがFlakyになったってハナシ

Last updated at Posted at 2022-12-06

はじめに

こんにちは。HRBrainでバックエンドエンジニアをしている@tonarinoheyです。
本記事はHRBrainのAdvent Calendar7日目に公開されました。

突然ですが皆さんは「クリスマス」って知っていますか。
僕は最近知りました。

先日2歳を迎えた息子も、保育園やYouTubeから「サンタさん」や「クリスマスプレゼント」なる概念を注入されてしまったらしく、無事カプセル化に失敗しています。

息子「ぶーぶ(自動車。空を飛ばないものだけを指す)、ちょーだい」

また我が家にトミカプレミアムが納車されるのか。うちのリビング、すでに大黒PA1みたいになってんのに。

クリスマスプレゼント、鮭フレークとかじゃ駄目かな。
子ども、鮭フレーク好きだし。僕も大好き。

Flaky Test is 何

フレークといえば、Flaky Testですよね。
そうなんだよ。考えるな。感じろ。

テスト結果が実行ごとに異なる不安定なテストケースはFlaky Testと呼ばれます。
Goで言えば、ローカルでgo testした時やCI実行時はすんなり通り、信じて送り出したテストコードがmainブランチのCI実行時でコケ、Slackで「mainでCI test落ちてるのでご確認お願いします:bow:」とメンションが飛んできてウワーッ(ちいかわ)になった経験が誰しも一度はあるでしょう。

えっ、無いですか?
僕だけですか、そうですか……。

そんなことより、クリスマスケーキ食って徹夜で金イクラサプライチェーン2の構築に尽力しようぜ……!

sliceを比較しただけなのに……

スマホを落としただけなのに3みたいに言うな。

例として、下記のようなプロダクトコードを考えましょう。前提として、プロダクトコードには不具合がない(仕様通りに動作する)ものとします。

manager.go
type Manager struct {
	ID        uint64
	UserID    uint64
}

type Managers []Manager

type ManagersByUserID map[uint64]Managers

func (g ManagersByUserID) GetUserIDs() []uint64 {
	userIDs := make([]uint64, len(g))
	i := 0
	for uid := range g {
		userIDs[i] = uid
		i++
	}
	return userIDs
}

テストコードは以下のように書きました。

manager_test.go
func Test_GetUserIDs(t *testing.T) {
	tests := []struct {
		name string
		want []uint64
		g    ManagersByUserID
	}{
		{
			name: "success",
			want: []uint64{111, 222},
			g: ManagersByUserID{
				111: {Manager{UserID: 111}},
				222: {Manager{UserID: 222}},
			},
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got := tt.g.GetUserIDs()
			if !reflect.DeepEqual(tt.want, got) {
				t.Errorf("want: %d, got: %d", tt.want, got)
			}
		})
	}
}

どっこい、上記テストケースsuccessはテスト実行ごとに成功したり失敗したりします。
信用ならない(Flaky)テストなんですね。

失敗した場合、下記のように、tt.wantgotとでsliceの要素の順番が異なると怒られました。

--- FAIL: Test_GetUserIDs (0.00s)
    --- FAIL: Test_GetUserIDs/success (0.00s)
        /manager_test.go:1293: want: [111 222], got: [222 111]

なんでなん

僕は書いたコードが期待挙動を示さない場合、ひとしきり碇シンジのように泣き叫んだ後4、心霊現象を疑います。
さっそく社内の寺生まれエンジニアに相談してみましたが、今回は"邪気"を感じないとのことでした。寺生まれってスゴイ……。

つまり僕のコードが悪いのです。
結論、テストコードでreflect.DeepEqualを用いてsliceを比較している箇所に原因があります。比較対象のうちgotManagersByUserIDをイテレートして取得した要素を詰めたsliceだからです。

Goはmapをイテレートした場合、keyおよびvalueの取得順序は一意に保証されませんよね5。これはmapを利用した際にイテレートで要素取得する順序の一意性に依存した実装をさせないための意図的な言語仕様です。

修正……してみよッ!(ハチワレ)

reflect.DeepEqualの利用に固執するなら、tt.wantの各要素がgot内に存在するかどうか確認する処理などを書いてあげれば良いかと思います。
プロダクトコード側でslice内要素の順番を保証する実装にするのであれば、一旦mapをイテレートしてkeyを取り出し、ソートした上で対応するvalueをmapから取得してsliceに詰めてあげてもいいですね。

とはいえ実装が冗長になってしまいますので、宗教上の理由などでassertパッケージ利用に抵抗がないならassert.ElementsMatchを使うとシンプルです。公式ドキュメントには下記のように書いてあります。

ElementsMatch asserts that the specified listA(array, slice...) is equal to specified listB(array, slice...) ignoring the order of the elements. If there are duplicate elements, the number of appearances of each of them in both lists should match.

や っ た ぜ。
ゆえに下記のようにテストコードを書き換えてあげられます。

func Test_GetUserIDs(t *testing.T) {
	tests := []struct {
		name string
		want []uint64
		g    ManagersByUserID
	}{
		{
			name: "success",
			want: []uint64{111, 222, 333},
			g: ManagersByUserID{
				111: {Manager{UserID: 111}},
				222: {Manager{UserID: 222}},
                333: {Manager{UserID: 333}},
			},
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got := tt.g.GetUserIDs()
			assert.ElementsMatch(t, tt.want, got) // 修正
		})
	}
}

また、Go 1.12以降ではmapをfmtによって印刷した際にkey順にソートされて印刷されますが、これは印刷時にソートされているだけなので、printデバッグ時などに錯誤しないよう注意が必要です。

明日のために

上記のようなミスは、もしテストコードを先に書いていれば、tt.wantgotの比較を書く際に、「このsliceの要素は順不同を許容するんだっけ?」という観点を持って防止できたかもしれないと思いました。
テスト→プロダクトコードの順に実装していればreflect.DeepEqualを書いたタイミングで、「GetUserIDs()から得られるsliceの要素って順序保証する必要あったっけ……?」みたいな仕様に対する疑問を持ち、適切なテストを書けたかも……(もちろん、それはそれでテスティングフレームワークに対する理解が必要なのですが)。
そうしたら、プロダクトコードを書く前に、仕様上の不確実性(が存在していた場合)を洗い出せたかもしれませんしね。

……かもかも言ってないで、テストもっと書くぞ! 

おわりに

じゃっ、僕はトイザらス行ってくるから! 
スカイラインGT-R(R34)6を買ってきます。

それではさようなら。

追伸

HRBrainは、3つのValue(Intensity、Take ownership、Power to the team)に共感し、オーナーシップを持ってプロダクトとビジネスを成長させたいという方を常に求めています。

中途採用だけでなく、新卒採用も積極的に行っています。
ぜひ下記リンク先にて詳細をご参照ください。
https://www.hrbrain.co.jp/recruit

  1. 神奈川県横浜市鶴見区の首都高速道路神奈川5号大黒線上にあるパーキングエリア。リアル湾岸ミッドナイトな人たちに根強い人気がある。

  2. スプラトゥーン3「サーモンラン」の話。しっかりとデスして参ります。

  3. 志駕晃による推理小説シリーズ。2022年現在、2作目まで『リング』で有名な中田秀夫監督によって映画化・公開されている。最新作の副題が『戦慄するメガロポリス』で未読なんだけど、絶対スマホ落とす以外にもやらかしてるだろこれ。

  4. 動け、動いてよ! 今動かなきゃ、何にもならないんだ!

  5. https://go.dev/doc/go1#iteration

  6. 昔の車。中古市場でちょっと意味がわからないくらい高騰している。

40
5
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
40
5

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?