はじめに
こんにちは!株式会社iimonにてフロントエンドエンジニアをしております、まつむらです!
今年もジメジメした季節がやってきますね。。
ただ、そんな湿度をも吹き飛ばすアツいムーブメントが弊社では起きています!
何がそんなにアツいのかというと、テストコード周りの改革がアツアツです!
- そもそもテストコードの工数も考慮して工数を出すようにする(極々当たり前ですが、、)。
- みんなで過去のコードに対するテストコードを書く。
- エンジニア合同勉強会にて全員で同じ動画を見ながらテストについて考える。
- テストカバレッジを上げる。
さて、テストコード熱が高いとは言ったものの、お恥ずかしながら、入社するまでのエンジニア人生でテストコードを書いたことがありませんでした。
(Excel等でテストケースはそれなりに書いてきたけど、、)
せっかくテストコードを学ぶのであれば、我らがEMイチオシのTDDについても学んでいければいいのかなと。
しかし、いざ実践しようとした際に、慣れないことから始めたのでとっつきにくさがありました。
そこで、今回は、「なんちゃって」でもいいから少しずつ慣れていって、「TDDはこういうものか!」というところに辿り着けたらいいな……!という実際の記録になります!
そもそもTDDってなんぞや?
TDDと聞いて最初は某ネズミの国の新エリアの略称かと思いましたが、そんなことはありませんでした。
コードの安全性が担保される夢の国への招待状のようなものと捉えればあながち間違ってないのかも……?(そんなわけはない)
TDDとは、Test-Driven Development、テスト駆動開発の略称で、テストを先に書いて、そのテストを通すためだけのコードを書く!といった開発手法になります。
これによって何が嬉しいのかと言うと、以下のものが挙げられるのかなと思います。
- テストがしやすくなる(当たり前ですが)
- テストがあることで、設計のイメージがつきやすくなる。
- 実装中にリファクタリングをする際に臆せず挑める!
- 壊れた際にいち早く検知できる!!
- 段階を踏んで実装することでPRサイズが小さくなる!!(※諸説あり。筆者の体感です。)
質とスピードがトレードオフの関係性というものを真っ向から突っぱねられる概念でもあったりします。
質が上がれば実装がしやすくなるので開発スピードが上がる。
開発スピードが上がればリファクタリング等のコードの改修に時間を充てられるので質が上がる。
この2つの良いサイクルを回していくのが真髄のようです。
また、開発の過程でRed-Green-Refactorを繰り返すことも特徴です。
Red: 失敗するテストを書く
Green: テストを通すためだけのコードを書く。
Refactor: 通すためだけのコードでは粗雑になりがちなので、リファクタリングをしてコードを綺麗にしていく
テスト落として、それをクリアできるようなものを書いて、綺麗にしていく、、そういうサイクルで進めていくのがTDD……
なるほど、よく分からん!
成功させるように書く、失敗なら落ちるパターンを網羅するのがテストじゃないの?という先入観があったため、「落ちるのが分かりきっているテストを書く」ということにイマイチ理解が追いつきませんでした。
そもそも実装してないもののテストを書くってなんぞや??とこちらも少し困惑してました。
また、理屈では分かるけど、質とスピードの両立って難しくない??という疑問もありました。
なので、ちょっとだけ実装して、ちょっとテストを書く。という手法から少しづつテストに慣れて、シフトしていくことにしました。
慣らし運転、始めました
では、実際の事例を元にこういう風に進めたよという過程を載せていこうかと思います!
※流石にそのままのコードを載せるわけにもいかないので、あくまで雰囲気だけ感じていただければ幸いです。
チケット内容「サービス区分Charlieを作るので、それを契約された方向けに別のフォーマットでcsvを吐き出す機能を追加してほしい。」
はい、要件がシンプルで助かりますね(?)
AlphaとBravoのサービス区分は既に実装済みなので、ここに足すだけで済みそうです。
ただ、以下の制約がのしかかっていました。
ただし、AlphaとBravoのテストコードは実装されていないものとする。
ナンテコッタ……
また、これらはFactory パターンで出し分けされている状況です。
つまりこのような形。
// src/csv/CsvWriterFactory.ts import { SAMPLE_1, // sample1.com SAMPLE_2, // sample2.com } from '@/const'; import { Sample1CsvWriter, Sample1ServiceAlphaCsvWriter, Sample1ServiceBravoCsvWriter, Sample2CsvWriter, Sample2ServiceAlphaCsvWriter, Sample2ServiceBravoCsvWriter, } from '.'; type CsvWriterInstanceType = | typeof Sample1CsvWriter | typeof Sample1ServiceAlphaCsvWriter | typeof Sample1ServiceBravoCsvWriter | typeof Sample2CsvWriter | typeof Sample2ServiceAlphaCsvWriter | typeof Sample2ServiceBravoCsvWriter; type ContractType = 'serviceAlpha' | 'serviceBravo' | ''; /** * CSV書き込みファクトリー * 対応サイトごとに出し分けを行う。 * サービス区分によっても分かれる。 * ただし、指定がなければデフォルトの物を返す。 */ export class CsvWriterFactory { static getInstance( contractType: ContractType ): InstanceType<CsvWriterInstanceType> | undefined { const host = window.location.host; if (host === SAMPLE_1) { if (contractType === 'serviceAlpha') { return new Sample1ServiceAlphaCsvWriter(); } if (contractType === 'serviceBravo') { return new Sample1ServiceBravoCsvWriter(); } return new Sample1CsvWriter(); } if (host === SAMPLE_2) { if (contractType === 'serviceAlpha') { return new Sample2ServiceAlphaCsvWriter(); } if (contractType === 'serviceBravo') { return new Sample2ServiceBravoCsvWriter(); } return new Sample2CsvWriter(); } } }
ごちゃっとしてますね、、!
ifが増えたら収拾がつかないことになりそうです。
実装前にリファクタリングしたい……。
しかし、これで現状動作確認が取れている以上、テストコード無きリファクタリングはNG。
なので、まずはここのテストを書くことにしました。
// tests/src/csv/CsvWriterFactory.test.ts import { SAMPLE_1, // sample1.com SAMPLE_2, // sample2.com } from '@/const'; import { CsvWriterFactory } from '@/csv/CsvWriterFactory'; import { Sample1CsvWriter, Sample1ServiceAlphaCsvWriter, Sample1ServiceBravoCsvWriter, Sample2CsvWriter, Sample2ServiceAlphaCsvWriter, Sample2ServiceBravoCsvWriter, } from '@/lib/csv'; // テスト用のwindow.locationを設定する関数 const setWindowLocation = (href: string) => { Object.defineProperty(window, 'location', { value: { href, }, writable: true, }); }; describe('CsvWriterFactory getInstance', () => { describe('該当しないドメインの場合', () => { beforeAll(() => { setWindowLocation(''); window.location.host = ''; }); test.each` service ${''} ${'serviceAlpha'} ${'serviceBravo'} `('何も返さないこと ($service)', ({ service }) => { expect( CsvWriterFactory.getInstance(service) ).toBeUndefined(); }); }); describe('sample1.comの場合', () => { beforeAll(() => { setWindowLocation(`https://${SAMPLE_1}`); window.location.host = SAMPLE_1; }); test('Sample1CsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('') ).toBeInstanceOf(Sample1CsvWriter); }); test('Sample1ServiceAlphaCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceAlpha') ).toBeInstanceOf(Sample1ServiceAlphaCsvWriter); }); test('Sample1ServiceBravoCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceBravo') ).toBeInstanceOf(Sample1ServiceBravoCsvWriter); }); }); describe('sample2.comの場合', () => { beforeAll(() => { setWindowLocation(`https://${SAMPLE_2}`); window.location.host = SAMPLE_2; }); test('Sample2CsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('') ).toBeInstanceOf(Sample2CsvWriter); }); test('Sample2ServiceAlphaCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceAlpha') ).toBeInstanceOf(Sample2ServiceAlphaCsvWriter); }); test('Sample2ServiceBravoCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceBravo') ).toBeInstanceOf(Sample2ServiceBravoCsvWriter); }); }); });
テストが通っているようなので、これでリファクタリングに着手できそうです。
今回は、今後にもパターンが増えることを想定してif-mapで置き換えることにしました。
// src/csv/CsvWriterFactory.ts import { SAMPLE_1, // sample1.com SAMPLE_2, // sample2.com } from '@/const'; import { Sample1CsvWriter, Sample1ServiceAlphaCsvWriter, Sample1ServiceBravoCsvWriter, Sample2CsvWriter, Sample2ServiceAlphaCsvWriter, Sample2ServiceBravoCsvWriter, } from '.'; type CsvWriterInstanceType = | typeof Sample1CsvWriter | typeof Sample1ServiceAlphaCsvWriter | typeof Sample1ServiceBravoCsvWriter | typeof Sample2CsvWriter | typeof Sample2ServiceAlphaCsvWriter | typeof Sample2ServiceBravoCsvWriter; type ContractType = 'serviceAlpha' | 'serviceBravo' | ''; const sample1CsvWriterWrapper = new Map< ContractType, InstanceType<CsvWriterInstanceType> >([ ['serviceAlpha', new Sample1ServiceAlphaCsvWriter()], ['serviceBravo', new Sample1ServiceBravoCsvWriter()], ['', new Sample1CsvWriter()], ]); const sample2CsvWriterWrapper = new Map< ContractType, InstanceType<CsvWriterInstanceType> >([ ['serviceAlpha', new Sample2ServiceAlphaCsvWriter()], ['serviceBravo', new Sample2ServiceBravoCsvWriter()], ['', new Sample2CsvWriter()], ]); const CsvWriterWrapper = new Map([ [SAMPLE_1, sample1CsvWriterWrapper], [SAMPLE_2, sample2CsvWriterWrapper], ]); /** * CSV書き込みファクトリー * 対応サイトごとに出し分けを行う。 * サービス区分によっても分かれる。 * ただし、指定がなければデフォルトの物を返す。 */ export class CsvWriterFactory { static getInstance( contractType: ContractType ): InstanceType<CsvWriterInstanceType> | undefined { const host = window.location.host; const wrapper = CsvWriterWrapper.get(host); if (!wrapper) { return; } return wrapper.get(contractType)!; } }
さて、構造自体が大きく変わってしまいました。
ただ、挙動自体は変わっていないことがテストコードにて担保されているので、問題はありませんでした。
というと少し語弊がありますが、、
実際にはいじった時にテストが落ちて、それを通るようにするにはどうすればいいのか、実行->修正を繰り返してました。
リファクタリングを進めるにあたってゴールが明確になっている・正解が用意されているため、自然と軌道修正されていく感じがしました。
なので、本来のRed-Greenとは少しズレるような気がしますが、こういう感じのサイクルで進めていくのかなという雰囲気は掴めました!
テストコード素敵やん。。。
と、テストコードの恩恵を受けつつ、いい感じに整理ができたので、いよいよ本題、サービスCharlieを追加していこうと思います。
なんちゃってTDDで初めてみる
とはいえ、流石にいきなりテストを落とすところから始めるというのがちょっとイメージがつきませんでした。
まだ作っていないものを呼ぼうとしてもそりゃ落ちるじゃないですか、、、
それにVSCodeにも存在しないものをimportするなと怒られるし。。。
なので、空っぽのclassだけ置いて、テストを通すことを目指してみました。
とりあえず、箱だけ用意してCsvWriterFactory.test.ts
のimportへ追記。
合わせてテストも追加してみます。
// tests/src/csv/CsvWriterFactory.test.ts import { SAMPLE_1, // sample1.com SAMPLE_2, // sample2.com } from '@/const'; import { CsvWriterFactory } from '@/csv/CsvWriterFactory'; import { Sample1CsvWriter, Sample1ServiceAlphaCsvWriter, Sample1ServiceBravoCsvWriter, Sample1ServiceCharlieCsvWriter, // 追加 Sample2CsvWriter, Sample2ServiceAlphaCsvWriter, Sample2ServiceBravoCsvWriter, } from '@/lib/csv'; // ~省略~ describe('CsvWriterFactory getInstance', () => { // ~省略~ describe('sample1.comの場合', () => { beforeAll(() => { setWindowLocation(`https://${SAMPLE_1}`); window.location.host = SAMPLE_1; }); test('Sample1CsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('') ).toBeInstanceOf(Sample1CsvWriter); }); test('Sample1ServiceAlphaCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceAlpha') ).toBeInstanceOf(Sample1ServiceAlphaCsvWriter); }); test('Sample1ServiceBravoCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceBravo') ).toBeInstanceOf(Sample1ServiceBravoCsvWriter); }); test('Sample1ServiceCharlieCsvWriterを返す', () => { expect( CsvWriterFactory.getInstance('serviceCharlie') ).toBeInstanceOf(Sample1ServiceCharlieCsvWriter); }); }); // ~省略~ });
getInstanceの引数にserviceCharlie
を追加していないのでVSCodeにめちゃくちゃ怒られてますが、お構いなしにテストを実行!
リテラル型で引数を制限しているので落ちますね、これは想定通り。
リテラル型を更新します!
// src/csv/CsvWriterFactory.ts // 'serviceCharlie'を追加! type ContractType = 'serviceAlpha' | 'serviceBravo' | 'serviceCharlie' | '';
これで引数エラーはなし、VSCodeにも怒られていない!再度テストを実行!
Factory側の実装がないので当たり前ではあるのですが、落ちます!
でも時にはその当たり前を確認することが大事だったりもします。
では本当に実装すれば動くのか?ということで実装します!
リファクタリングにてsample1CsvWriterWrapperに追加すれば良くなったので、そこへ追加します。
// src/csv/CsvWriterFactory.ts import { Sample1CsvWriter, Sample1ServiceAlphaCsvWriter, Sample1ServiceBravoCsvWriter, Sample1ServiceCharlieCsvWriter, // 追加 Sample2CsvWriter, Sample2ServiceAlphaCsvWriter, Sample2ServiceBravoCsvWriter, } from '.'; type CsvWriterInstanceType = | typeof Sample1CsvWriter | typeof Sample1ServiceAlphaCsvWriter | typeof Sample1ServiceBravoCsvWriter | typeof Sample1ServiceCharlieCsvWriter // 追加 | typeof Sample2CsvWriter | typeof Sample2ServiceAlphaCsvWriter | typeof Sample2ServiceBravoCsvWriter; const sample1CsvWriterWrapper = new Map< ContractType, InstanceType<CsvWriterInstanceType> >([ ['serviceAlpha', new Sample1ServiceAlphaCsvWriter()], ['serviceBravo', new Sample1ServiceBravoCsvWriter()], ['serviceCharlie', new Sample1ServiceCharlieCsvWriter()], // 追加 ['', new Sample1CsvWriter()], ]);
これで心当たりがありそうな箇所は埋められました。
そしてテストを実行、、
無事に通りました!
Factory自体は通ったので、あとは中身の実装をしていくだけになりそうです!
まだやることがあるにしても、FactoryパターンのテストコードだけでもPRが出せる状態にはなったので、レビュー依頼を出しました。
テスト駆動で得られたもの
テスト駆動で進めると嬉しいことは分かりましたが、「テストだけ先に書く」というのがpingとこなくて、ある程度出来上がったらテストを書くという手法で進めがちでした。
そこでEMに相談したところ、「テストを先に書くことで、そもそもそのテストの書き方が正しいのかを検知できる(意訳)」とアドバイスをいただいて腑に落ちました。
テストを先に書かないことで、起こりうるデメリットとしては、メリットの逆、つまりは以下のことが起こりうるのではないかなと。
設計のイメージがつきづらく、テストがしづらいコードになる。
- 密結合・低凝集になりやすい
そもそもテストが正しく書かれているかを検知できない。
- テストコード自体のバグなのか、実装のバグなのか分からない。
- テストの書き方が悪いのか、そもそも論で導入ミスなのか、色々と考慮する必要性が出てくる。
今回は説明を省きましたが、Factoryの先、Sample1ServiceCharlieCsvWriter.ts
に処理を書いていく上で、上記のことを意識して書き進めることができました。
特に、疎結合・高凝集にするという意思を強く持ちながら実装できたのかなと思います。
まずは枠だけ実装・テスト。
次に単純なIn/Outだけの機能を持ったものを実装しながらテストを拡充。
テストが書きにくいなと感じたら、そもそもの実装が肥大化していないか?責務は正しいか?と振り返りながら実装することができました。
結果的に何が嬉しかったのか
今回、なんちゃってという雑な形でもTDDに触れてみて、結果的に何が嬉しかったのかというと、安全にリファクタリングが行えた点ですかね。
実際に動いていたコードをリファクタリングする場合に、ズレそうになっても自然と正しい動作をするように導かれていったように感じました。
また、テストファーストで書くということは、いい意味でテストが通るような書き方になるので、密結合・低凝集なコードから疎結合・高凝集なコードに自然と誘導され、複雑なコードになりにくいようにも感じました。
当然、複雑性が下がるので、可読性が上がるというのが嬉しかったです!
見通しが良くなることで、追加実装等があってもリファクタリング済みなので、分岐なんか辿らずに1行足すだけで済んだのが結構大きいなと。
おや、質とスピードって両立できるような、、?
おわりに
最初は納期第一!テストコードは後で書く!のようなスタンスが蔓延していました。
そして// Todo: リファクタリングはあとでやる
というコメントばかりが増えてく、、、(その間に積み重なる実装、、)
リファクタリングをするにしてもちょっと動作が怪しい箇所、色々な場所で使用されているために手がつけにくい箇所が多々ありました。
ただ、テストコードを書くぞ!という流れが生まれてからは、TDDとまではいかないものの、少しづつテストに依存するようになってきたのかなと個人的には感じてます。
慣れてしまうと、テストコードがないと逆に不安になるという副作用はあるのかもしれませんね笑
TypeScriptに触れてから、型がないと不安でしょうがなくなる現象に近いかもです!
型だけでなくテストにも依存することで、より安全かつ読みやすく、そして責務を明確に書けるようになっていくのかなと。
まだまだTDDと呼ぶには程遠いですが、少しづつ慣らしていって気がついたらTDDしてた!と言えるように精進して参ります!
最後までお付き合いいただきありがとうございました!
弊社ではエンジニアを募集しております。
ぜひカジュアル面談でお話ししましょう!
ご興味ありましたら、ご応募ください!