20 万行超のコードベースをテストせずにリファクタリングリリースした話 - MonotaRO Tech Blog

こんにちは、鈴木です。 20 万行を超えるアプリケーションのほとんど全てのソースコードを変更し、テストを行わずに本番リリースしました。 「それってテストいるんですか？」問題 いきなりですが質問です。ソースコードを 1 バイトでも変更したら再テストする必要はあるでしょうか。「絶対に再テストすべき」という方もいれば、「状況によるしケースバイケースかな・・」という方もいらっしゃると思います。 ケースバイケースと考える方は、どのような場合にテストを行わなくて良いと考えるでしょうか。例えば、コメント内の誤字を修正した場合はどうでしょうか。ローカル変数の名前を typo していたので修正した場合、デッドコードを削除した場合はどうでしょうか。 こんなことがありました ある日、Python のソースコードを眺めていると、「# $Id」のような CVS 時代のコメントがありました。いまやソースコードは Gi

[zumrud]

「ASTが変わらないことを確かめる」という手段のテストをしたことになるのでは？

[uva]

CIで自動テストを回していない環境だと推測できる内容を会社のtech blogに書くのはどうなんだろう。書いてる人が気にならないのならいいんだけどさ

[takuyachallenge]

これはリファクタリングなのか…？あと、単体テストがあるならやればいいのでは…？

[otoan52]

AST木の話はわりと面白い。babel通して同じコードだから大丈夫ーみたいな事はやった事あるな。ただ、それ、リファクタリングやない、クリーニングや。

[knjname]

変数名がASTに含まれていないと主張しているようにみえるブコメがあって笑った。結局そういう変更はやってないっていってるしね。これはフォーマッタで直されたらテストが必要か？という話でもあるんじゃ

[efcl]

https://github.com/azu/js-ast-hash JavaScript/TypeScript向けのASTをハッシュ値にするツールを書いた

[send]

リファクタリングではない

[fashi]

結局並び替えや変数名変えるとか軽微な修正はしなくて、冗長なコメント削除しただけで、「行数を減らすことができました」って言われても

[naari_3]

CIはない文化の会社なの？

[Magicant]

「確認するための小さなツールを作成し、それを用い」たのであれば、それはテストをしたといふことでは?

[juve534]

"ASTが変わらなければテストはいらない" という落とし所はいいな。自分たちは暗黙的なスキップをしてしまっているので、ちゃんとできるようにしていきたいな。

[tpircs]

内部構造変えないんだからリファクタリングじゃないのでは。リファクタリング対象でないことを確認した、ならわかる。

[mizoguche]

"結論は「抽象構文木 (AST: Abstract Syntax Tree) が変化しない変更であればテスト不要」"

[kkobayashi]

コード整形ってリファクタリングに入るの？

[UDONCHAN]

タイトルが悪い

[masaru_b_cl]

JetBrainsのお世話になろう

[do_su_0805]

いい話。ちゃんと抽象構文木という基準をもって判断してる

[muddydixon]

なるほど

[d6rkaiz]

リファクタリングと書かれると違和感あるけどASTで比較するのは良いと思った。

[veleno]

それ、リファクタリングじゃなくない？ https://ja.wikipedia.org/wiki/%E3%83%AA%E3%83%95%E3%82%A1%E3%82%AF%E3%82%BF%E3%83%AA%E3%83%B3%E3%82%B0_(%E3%83%97%E3%83%AD%E3%82%B0%E3%83%A9%E3%83%9F%E3%83%B3%E3%82%B0)

[ryuzee]

リファクタリングというよりクリーニング？の話だけど、面白い。人海戦術のテストが一番高コストだから、そもそもテスト書こうよという話ではある

[fumisan]

うーん、結局、前後でソースのロジックが変わっていないことをハッシュで検査しているので、それはリグレのテストと同じなのではーとおもうぞ。

[oxon]

Python であれば import の順序で挙動は変わりうるし、変数名のみリファクタリングしても locals と format の組み合わせでこける可能性もあるのでテスト必須だと思う。

[june29]

「テスト」と「リファクタリング」がそれぞれなにを指すのか、解釈のズレによって著者と読者の間ですれちがいを生じていそう…。最初の PR は大きすぎると思う。

[snowcrush]

大規模にコメントの変更を行う際のテクニックとしては有用だと思うが、活用できる場面はそんなになさそう。

[nacika_inscatolare]

ASTが変化する条件だいぶシビアだと思うけど、縛りプレイでリファクタリングしてる状態になってだいぶきついと思うw

[zonu_exe]

最初「テストする・しない」から？だった。理想ではPR単位でユニットテストが勝手に動いてほしい。古いコードの不必要なコメントを掃除するのはよくわかるが、今後AST変化しないコード変更する用事があるかは疑問。

[ene0kcal]

ASTの変化なしでテスト不要とか、今後流行りそう。ほかの言語ではどれだけ可能なのか知りたい。なお、個人的には関数単位でASTの同一性が知りたい。関数をリファクタリングする過程等で参考にしたいので。

[foostan]

CIの時間を短縮する手段として使えそう

[rokujyouhitoma]

そもそもなぜテストをしないのか

[nobuoka]

良い。 んだけど 「AST が変化していないことを検査している」 のでテストしていると言っていいのでは？ (クソリプ)

[kazuhooku]

AST変わらなくても変数名変えたら動作変わる「可能性」はあるでしょ。例外のメッセージだって変わるわけだし。テストがめんどくさい場合の線引きとしては妥当だと思う

[Haaaa_N]

ASTのdiffがGitHubのpull requestにCI的に出てきたら有用っぽい

[amedama41]

重要なのは品質を何で担保するかだからテストに固執する必要はないよね。 AST比較するのは、テストがない既存コードにフォーマッタかけた結果のチェックに良さそう

[nippondanji]

モノタロウさん、かなり買い物でお世話になってるんで頑張ってほしい。

[hazisarashi]

テストのコストどんだけ高いんだろう…

[uturi]

抽象構文木の話はなかなか興味深かった。リファクタリングしてもASTが変わらない、というテストは規模が大きくなると大変そう。

[wyamamo]

昔、テストしなければ派の斜め上の上司に諭されたことを思い出しました。AST 調べてみます

[Watson]

リファクタリングとは・・・

[sig_zone]

世の中にはコードをコメントアウト(ダミーJOB化)するだけで本番障害発生させた奴も居るんだゾ。