[project-sequencer-statemachine] StoreをDIするように変更

[sigprogramming]

内容

StoreをDIするようにします。

関連 Issue

• ソング：シーケンサーのプレビュー処理をステートパターンで実装して置き換える #2403

その他

[voicevox-preview-pages]

🚀 プレビュー用ページを作成しました 🚀

• 📝 エディタ
• 📖 Storybook

更新時点でのコミットハッシュ：b9cd67a

[Hiroshiba]

すみません、ちょっと一旦方針だけ確認したく…！

スタートマシンできれいに単体テストできる構成だったのが、しづらくなりそうですね…！
そろそろテストを実装できればとも考えているのですが、どういう方針が良さそうでしょうか👀
（ソングエディタ全体でe2eテストを書く or スタートマシンで単体テストを書く or テストを諦める、など）　

ちなみにstoreをDIする形でも単体テストは可能です🙏
（storeをマックすれば良い）
ただシグネチャが変わる度にモック変えないとなので、単体テスト時はそれなりに不便かもです。

[sigprogramming]

@Hiroshiba
各ステートのロジックはコンポーネントのロジックでもあります。
（ステートを切り替えるとコンポーネントの振る舞いも切り替わる）
なので、単体テストよりコンポーネントテスト・E2Eテストの方がもしかしたら合っているかもしれません。
（テスト周り、あまり詳しくないので自信ないですが…）
https://ja.vuejs.org/guide/scaling-up/testing#unit-testing

単体テストを行うのであれば、storeのモックと、ブラウザのAPI（requestAnimationFrame、MouseEventなど）のモックが必要になると思います。
（storeのモックの作成は以前の形でも行う必要があると思います）

[Hiroshiba]

テストはちょっと僕もわかってないのですが、検証したい項目があり、効果が大きいとこでテストするのが良い気がしています！
ステートマシン単体でテストを書いておけば、ステートマシン自体のリファクタリングやバグ発見が容易にはなると思います。
ステートマシンがどれくらい壊れやすいか・どれくらい変更されやすいか次第かもです。

ステートマシンの単体テストの正常系は例えば、クリックしたらのノートが置かれる（実際にVuex状態変数が変わるのではなく、モック関数を仕込んでおく）、ノートを範囲選択したらステートが移動し想定した状態変数になる、何もないところを範囲選択しても状態が遷移しない、でも選択中に無を範囲選択するとIdleに戻る、とか･･･！
（どの状態に遷移しているかはe2eテストでは得られない）

ちなみにMouseEventのmockは相当簡単（オブジェクト作るだけ）なはずで、requestAnimationFrameは単体テスト時はいらないかも（というより要らないように作る）です！
e2eにするならどんな形でもstoreのモックは不要そう。store＋ステートマシンの複合テストをする際はmockがあっても良いかも。（とても大変そうだけど）

なんだかんだ言ってるけれども、結局ステートマシンがどれぐらい壊れやすいのものなのか先見の明がないので、テスト書くべきかどうかわからないんですよねー！！
テスト書くなら絶対に1枚挟んでおいた方が良い（PR前のDIしている形）のは知ってるのですが。。という感じでコメントしました！

うーーーーーーん。
おすすめは今の冗長構成にしといて、やっぱりめんどくさいなとなった＆テストを書かなくて良いなとわかったら外すとかでも良いかもです！
まあでもそんなに戻すのも難しくなさそうなので、今もうすでにめんどくさいとかであればstoreと密結合させちゃってもまあという感じです！！（きっと２年後に答えがわかる･･･！）

[sigprogramming]

@Hiroshiba
各ステートの処理はUXに関わるものなので、そこそこの頻度で変更されると思います。
なので、おっしゃる通り単体テストを行った方がバグは防げると思います。

e2eにするならどんな形でもstoreのモックは不要そう。store＋ステートマシンの複合テストをする際はmockがあっても良いかも。（とても大変そうだけど）

Storeのモックはstate, getter, action, mutationなどをすべて実装しないといけないので大変ということでしょうか？
であれば、ステートマシンで使用する一部のstate, getter, actionのみを定義した型を作れば、useSequencerStateMachineするときはstoreのインスタンスをそのまま渡せて、テストでは一部のstate, getter, actionを実装したモックを渡す形にできると思います。
ひとまずその形にしてみました。 b9cd67a

[Hiroshiba]

Storeのモックはstate, getter, action, mutationなどをすべて実装しないといけないので大変ということでしょうか？

一応全部実装しなくても可能ではあるのですが、無理やり関数をモックで上書きする形になったりなので、結構不便なんですよね。。。

とりあえず1枚噛ませておけば形は何であれ OK だと思うので、今のプルリクエストの形でいい感じだと思いました！！
Vuex側のstoreの型を変えたらこっちも変更しなくちゃいけなく、回数が増えてきたら大変だと思うので微妙だなと思った時はご相談ください！

Pick<SingActions, "PLAY_PREVIEW_SOUND" | "COMMAND_ADD_NOTES" | ...>
みたいにすればシグネチャをごそっと取ってこれる･･･はず！（型パズルが必要ですが）

[Hiroshiba]

LGTM！！

useSequencerStateMachineで得られたいろんなインスタンスをprovide/injectする感じになりそう･･･でしょうか！
どんな感じの開発心地になるか楽しみです！！！

[Hiroshiba]

LGTM！！

useSequencerStateMachineで得られたいろんなインスタンスをprovide/injectする感じになりそう･･･でしょうか！
どんな感じの開発心地になるか楽しみです！！！

[Hiroshiba]

あ、もしかしたらmainにマージ可能な状態になった感じでしょうか？ 👀
マージ方針どうして行くのがいいのかなーと！

ちょっとパット考えた案まで：

• 一気に全部置き換えてマージする
  • mainブランチにこのブランチをマージした後、もしくはこのブランチでマージ作業をした後に。
  • 変更中にmainブランチのステート周りのロジックが変わるとマージがかなり大変になりうる
• 一旦mainにマージした後、ステート管理を2種類にして、徐々に置き換えていく
  • 一番合理的ではあるけど、どっちもある状態をメンテナンスするのが大変かもしれないのと、単純に計算量が2倍になる
  • 移行が時間かかりそうならこっちの方が良さそうかも

[sigprogramming]

@Hiroshiba
「一気に全部置き換えてマージ」が良いかなと思います！
（project-sequencer-statemachineで置き換えまで行って、その後mainにマージを考えています、置き換え自体は簡単にできると思います）

[Hiroshiba]

承知しました！！