ユニットテストでbeforeEachなどのセットアップを使う際の注意点と改善例
皆さんはユニットテストを書かれていますか。私は書いています。
ユニットテストを何かしらのテスティングライブラリやフレームワークを使う場合、以下のようなことを言われたことがありませんか。
- 共通の前処理は
beforeやbeforeEachに書くと同じことを繰り返さずに済むよ - 後処理も同様に
afterやafterEachに書くと良いよ
参考: jestの例はこちら。
確かにこれらのセットアップ関数などはとても便利なのですが、一方でいくつか留意した方が良いポイントを含んでいると思います。
今回はサンプルコードを見つつ、以下のような内容についてお話したいと思います。
-
beforeEachなどの共通処理を行う際の注意点 - それに対する改善提案
- 逆に
beforeEachを使っても良いかなぁ。と思うケースについて
追記 2024/08/20
本記事の内容をもとに以下でLTをさせていただきました。
サンプルコード
みなさんAWSは使ったことがありますか？
AWSにはたくさんの便利なサービスがありますが、一部のサービス名について間違えやすいものがあります。例えば、以下のようなサービス名です。
- Amazon EC2
-
AWS Security Hub
-
AmazonではなくAWSです - また
SecurityHubではなくSecurity Hubであることにも気をつけましょう
-
- Amazon CloudFront
- こちらは
Amazonですね - また
Cloud FrontではなくCloudFrontです。こっちはスペースがはいりません。
- こちらは
このようにサービス名を間違えてしまいやすいので、気付けるようなサンプルコードを書いてみました。
直接はテストケースに関係ないため折りたたみしています
// AWS 名の typo に気づけるように作ってみたサンプルクラス
export class AwsServiceNameValidator {
readonly prefix: string
readonly serviceName: string
// EC2 や S3 は Amazon から始まります。
readonly prefixAmazon = new Set(["EC2", "S3"])
// Security Hub や Lambda は AWS から始まります。
readonly prefixAws = new Set(["Security Hub", "Lambda"])
// Security Hub はスペースが入ります。逆にCloudFrontはスペースが入りません。
// 間違えやすいサービス名のセットです
readonly invalidSpace = new Set(["SecurityHub", "Cloud Front"])
constructor(prefix: string, serviceName: string) {
this.prefix = prefix
this.serviceName = serviceName
}
// Amazon はじまりなのか AWS はじまりなのかをチェック
isValidPrefix(): boolean {
// 本来 Amazon から始まるべきサービス名なのに AWS から始まっている場合はfalse
if (this.prefixAmazon.has(this.serviceName) && this.prefix === "AWS") {
return false
}
// 本来 AWS から始まるべきサービス名なのに Amazon から始まっている場合はfalse
if (this.prefixAws.has(this.serviceName) && this.prefix === "Amazon") {
return false
}
return true
}
// 間違えやすいスペースが入るのか、入らないのかをチェック
// Cloud Front は余計なスペースが入っているのでfalse
// SecurityHub は必要なスペースが入っていないのでfalse
isValidSpace(): boolean {
if (this.invalidSpace.has(this.serviceName)) {
return false
}
return true
}
}
さて、このクラスのテストをするのにあたってテストケースを考えてみましょう。
ユニットテストを書く
Step1: ベースとなるちょっと冗長なテストケース
まずは以下のようにテストケースを書いてみました。
import { AwsServiceNameValidator } from "./awsServiceNameValidator"
// 何回も同じ初期化を繰り返しているパターン
describe("AwsServiceNameValidator", () => {
describe("isValid Prefix", () => {
it("return true with prefix AWS and serviceName Security Hub", () => {
// arrange
const awsServiceNameValidator = new AwsServiceNameValidator(
"AWS",
"Security Hub"
)
// act
const shouldTrue = awsServiceNameValidator.isValidPrefix()
// assert
expect(shouldTrue).toBeTruthy()
})
})
describe("isValid Space", () => {
it("return true with serviceName Security Hub", () => {
// arrange
const awsServiceNameValidator = new AwsServiceNameValidator(
"AWS",
"Security Hub"
)
// act
const shouldTrue = awsServiceNameValidator.isValidSpace()
// assert
expect(shouldTrue).toBeTruthy()
})
})
})
他にテストケースが足りていないなど、言いたいことはありますが一旦こちらに書いてあるテストケースだけで考えてみましょう。
これらは AWS SecurityHub という入力に対して以下のようなテストを行っています。
- 接頭辞が
AmazonではなくAWSとなっていて正しい -
Security Hubというようにちゃんと正しいスペースが入っている（SecurityHubではない）
もしかすると、以下の部分について同じような初期化を何度もしているのが気になるかもしれません。（AwsServiceNameValidator が変更になった時の変更箇所も多いですよね）
const awsServiceNameValidator = new AwsServiceNameValidator(
"AWS",
"Security Hub"
)
Step2: beforeEachを使ってみたものの・・・
そうだ！同じような前処理はセットアップ関数にまとめれば良いって聞いたことがある！ ということで以下のようなコードを書いてみたくなるかもしれません。
// 次にbeforeEachを使っているパターン
// 一見スッキリして良さそうだけど・・・
describe("AwsServiceNameValidator", () => {
let awsServiceNameValidator: AwsServiceNameValidator
beforeEach(() => {
// 共通化して前処理もセットアップ関数にまとめてスッキリ
// と思うじゃん？
awsServiceNameValidator = new AwsServiceNameValidator("AWS", "Security Hub")
})
describe("isValid Prefix", () => {
it("return true with prefix AWS and serviceName Security Hub", () => {
// act
const shouldTrue = awsServiceNameValidator.isValidPrefix()
// assert
expect(shouldTrue).toBeTruthy()
})
})
describe("isValid Space", () => {
it("return true with serviceName Security Hub", () => {
// act
const shouldTrue = awsServiceNameValidator.isValidSpace()
// assert
expect(shouldTrue).toBeTruthy()
})
})
})
一見するとスッキリしていて良さそうですが、以下のような点に気をつける必要があります。
-
awsServiceNameValidatorという変数をテストケースの間で共有している- 一方のテストケースの条件を変更したい場合、他方のテストケースに影響が出てしまう
- 今回はたった2個しかないのでまだ良いですが、他にもテストケースが増えると、それだけ状態を共有してしまう
- 一方のテストケースの条件を変更したい場合、他方のテストケースに影響が出てしまう
- 確かに「同じコードを繰り返していた」を解消できた気がするけど、そもそもこの初期化処理は共通処理としてまとめるべきなのか
- 2つのテストケースで確かめたかった関心事は全然別物のはず
- サービスの接頭辞が正しいか
- サービス名に正しい形でスペースが入っているか
- 確かに「同じコード」を使っていたけど、それはまとめることがDRY原則と言えるのか
- 2つのテストケースで確かめたかった関心事は全然別物のはず
- スクロールしないと
beforeEachがどのような初期化を行っているのかがわからない- 安易にセットアップ関数でまとめると、下の方のテストケースでは結構上にスクロールしないと
awsServiceNameValidatorがどのような初期化を行っているのかわからない - さらにこれがネストしていると、どこで何をしているか非常にわかりにくい
- 後でテストを追加したり変更する人でもぱっと見てわかりやすい方が良い
- 以下の画像のような状態（今回は2つだけですが他にも沢山テストケースがあると、後のテストでは沢山スクロールしないと初期化の条件が見えない）
- 安易にセットアップ関数でまとめると、下の方のテストケースでは結構上にスクロールしないと
良いユニットテストの条件には「テストケース間が独立していて、どんな順番で実行されても問題ない」というものがあると考えています。
セットアップ関数はとても便利な反面、ある種テストケース間で状態を共有してしまい、テストケース間の独立性を損なう可能性があると思います。
ということで、次のようにテストコードを変更してみます。
Step3: ヘルパー関数を使ってみる
// ヘルパー関数(ファクトリー関数)
// クラスに修正が入ってもここだけ直せばいいし、テストケースの中身が見やすい
// 引数を省略してデフォルトの引数で初期化したインスタンスを返すのもよいかも
const makeAwsServiceNameValidator = (
args: ValidatorArgs
): AwsServiceNameValidator => {
return new AwsServiceNameValidator(args.prefix, args.serviceName)
}
describe("AwsServiceNameValidator", () => {
describe("isValid Prefix", () => {
it("return true with prefix AWS and serviceName Security Hub", () => {
// arrange
const awsServiceNameValidator = makeAwsServiceNameValidator({
prefix: "AWS",
serviceName: "Security Hub",
})
// act
const shouldTrue = awsServiceNameValidator.isValidPrefix()
// assert
expect(shouldTrue).toBeTruthy()
})
})
describe("isValid Space", () => {
it("return true with serviceName Security Hub", () => {
// arrange
const awsServiceNameValidator = makeAwsServiceNameValidator({
prefix: "AWS",
serviceName: "Security Hub",
})
// act
const shouldTrue = awsServiceNameValidator.isValidSpace()
// assert
expect(shouldTrue).toBeTruthy()
})
})
})
一見すると、一番最初のコードと変わらないように見えますが、以下のような利点があります。
-
AwsServiceNameValidatorに変更が入ってもmakeAwsServiceNameValidatorだけを修正すれば良い -
beforeEachで共有していた状態をテストケース間で共有せずに済む- 変更が必要になった箇所だけ引数を変更すれば良い
- テストが大きくなってきても、テストケースの中を見るだけでどのようなテストが行われているかがわかる
- 今回の例だと
makeAwsServiceNameValidatorがどのような初期化を行っているかが一目でわかる -
beforeEachを使った状態の共有よりメンテナンスがしやすい
- 今回の例だと
このテクニックや考え方は以下の書籍で作者の Roy Osherove さんが提唱されていました。
また、以下のような記事でも同様の内容を紹介されていますので、興味がある方はぜひ読んでみてください。
逆にセットアップ関数などを使っても良いのでは？という場合
DBなどの外部の依存の状態に嫌でも左右される場合 が主に該当すると思います。
一般的なWebシステムにおいては「DBを取り扱うクラス（モデルやリポジトリ）」があったり、DBの状態に依存するようなテストを書くことがあります。
フェイク（モック、スタブ）を駆使してユニットテストで書くよりも、コンテナ仮想化技術などで立ち上げたDBを使って、よりアプリの信頼度を高めたテストも書きますよね。（これはユニットテストというよりも統合テストにあたるのかもしれませんが）
そのような場合、以下のような前処理・後処理のために beforeEach や afterEach を使うのも良いと思います。
describe("createBlogPost", () => {
beforeEach(() => {
createDummyUser()
})
afterEach(() => {
rollBackTransaction()
})
it("return created blog post", () => {
// arrange
const blogPostData = {
title: "Hello, World!",
content: "This is my first blog post!",
user: "dummyUser"
}
// act
const createdBlogPost = createBlogPost(blogPostData)
// assert
expect(createdBlogPost.title).toEqual(blogPost.title)
})
})
DBが絡むテストであれば「他のテストケースに影響を及ぼさないように、逆に後処理でクリーンな状態に戻す」などの処理を行うのは一般的にあると思います。（むしろ状態を引きまわさないように帳尻を合わせる）
このように「どうやってもテストケース間で共有する必要のある外部の依存」については、セットアップ関数などを利用するのも良いと思います。
とはいえ、極力ユニットテストをしやすくなるように直接DBをやORMと関わるのは一部の関数やクラスだけにして、ビジネスロジックとDBの都合との分離は意識したいところですね。（DBにかかわらず、外部への依存を極力減らすことがテストしやすいコードになると思います）
まとめ
-
beforeEachなどのセットアップ関数の利用には注意が必要です。テストケース間で状態を共有してしまうリスクがあります - 状態を共有せずに済むよう、ヘルパー関数を使用する方法があります。テストケースの可読性向上、メンテナンス性向上に繋がると思います
- ただし、DBなどが絡む統合テストの場合にはセットアップ関数などが有効だと思われます。むしろ、テストケース間の状態を共有しないようにクリーンな状態に戻す処理を行うためです
The Art of Unit Testingはとてもおすすめの書籍ですが、時間がない方はMicrosoftのユニットテストベストプラクティスなどをみるだけでも作者の考え方に触れられると思います。
最後まで読んでいただき、ありがとうございました！
超余談
今回のように「AWSのサービス名を間違えたくないよ」という時のためにtextlintというツールのプラグインを作っています。
ご興味のある方は以下のリポジトリを見てみてください。
Discussion