Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPIのスナップショットテスト追加&不要なQueryを省く #992

Merged
merged 5 commits into from
Jan 18, 2024

Conversation

Hiroshiba
Copy link
Member

@Hiroshiba Hiroshiba commented Jan 8, 2024

内容

こちら↓のプルリクエストをマージしたとき、APIが意図しない形で変わりました。

クエリパラメータだったのがリクエストボディに変わった感じです。ちなみにそれを @sabonerune さんが気づいて直してくださいました。

APIの形が(FastAPIの知識不足で)意図せず変わってしまうことがわかりました。
結構怖いので、APIの形であるOpenAPIのスナップショットテストを追加しました。

ついでに知識をつけて不要なことがわかった= Query()をいくつか削除しています。

その他

ちなみに、もし #826 の変更で意図せずAPIが変わっていたところは、OpenAPIの差分としてはこんな感じで表示されます。

image

たしかにrequestBody内にあったのがin: query内に移ってしまってるのですが、この差分を見れば問題に気づけたかというと・・・・・・・・ 😇
まあでもAPIの形を意識できるので、良いかなと。

@Hiroshiba Hiroshiba requested a review from a team as a code owner January 8, 2024 10:27
@Hiroshiba Hiroshiba requested review from y-chan and removed request for a team January 8, 2024 10:27
Copy link

github-actions bot commented Jan 8, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 511 317 coverage-38%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/cancellable_engine.py 94 72 coverage-23%
voicevox_engine/core_adapter.py 60 17 coverage-72%
voicevox_engine/core_initializer.py 59 30 coverage-49%
voicevox_engine/core_wrapper.py 225 159 coverage-29%
voicevox_engine/dev/core/init.py 2 0 coverage-100%
voicevox_engine/dev/core/mock.py 36 8 coverage-78%
voicevox_engine/dev/tts_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/tts_engine/mock.py 28 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifest.py 34 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 12 0 coverage-100%
voicevox_engine/engine_manifest/init.py 3 0 coverage-100%
voicevox_engine/library_manager.py 92 5 coverage-95%
voicevox_engine/metas/Metas.py 34 0 coverage-100%
voicevox_engine/metas/MetasStore.py 18 6 coverage-67%
voicevox_engine/metas/init.py 2 0 coverage-100%
voicevox_engine/model.py 164 9 coverage-95%
voicevox_engine/morphing.py 71 46 coverage-35%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 13 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 80 2 coverage-98%
voicevox_engine/preset/init.py 4 0 coverage-100%
voicevox_engine/setting/Setting.py 11 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 17 0 coverage-100%
voicevox_engine/setting/init.py 3 0 coverage-100%
voicevox_engine/tts_pipeline/acoustic_feature_extractor.py 34 0 coverage-100%
voicevox_engine/tts_pipeline/kana_converter.py 88 1 coverage-99%
voicevox_engine/tts_pipeline/mora_list.py 7 0 coverage-100%
voicevox_engine/tts_pipeline/text_analyzer.py 146 6 coverage-96%
voicevox_engine/tts_pipeline/tts_engine.py 177 11 coverage-94%
voicevox_engine/user_dict.py 145 12 coverage-92%
voicevox_engine/utility/init.py 5 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 13 0 coverage-100%
voicevox_engine/utility/path_utility.py 35 9 coverage-74%
voicevox_engine/utility/run_utility.py 10 7 coverage-30%
TOTAL 2288 718 coverage-69%

Copy link
Member Author

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIの形がどう変わったかをチェックしやすいようにこちらマージします!

@Hiroshiba Hiroshiba merged commit 4fe753d into VOICEVOX:master Jan 18, 2024
3 checks passed
@Hiroshiba Hiroshiba deleted the 不要なQueryを省く branch January 18, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant