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

追加: テキスト分析のテスト #970

Merged
merged 4 commits into from
Jan 3, 2024

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Jan 3, 2024

内容

テキスト分析のテストを追加

現在のテキスト分析モジュール text_analyzer は中心的関数である text_to_accent_phrases() をテストしていない。
またこの関数は pyopenjtalk へ 直接 call 形式で依存しており、異常系テストが困難である。
また現在の VOICEVOX ENGINE は text_analyzer の異常系出力が正常系に乗って下流モジュールに伝播する仕様となっているが、そのテストもなされていない。
すなわち、テキスト分析関連テストに関する改良の余地が多分にある。

よってテキスト分析のテスト関連改良を提案します。

具体的には次の改良を提案する:

  • pyopenjtalk の DI (text_to_features 引数の追加)
  • text_to_accent_phrases() の正常系テスト
  • text_to_accent_phrases() の異常系テスト (xx 音素への応答)
  • 下流 Phoneme クラスの異常系テスト (xx 音素への応答)

関連 Issue

ref #894

@tarepan tarepan requested a review from a team as a code owner January 3, 2024 06:39
@tarepan tarepan requested review from Hiroshiba and removed request for a team January 3, 2024 06:39
Copy link

github-actions bot commented Jan 3, 2024

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 511 327 coverage-36%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/cancellable_engine.py 93 72 coverage-23%
voicevox_engine/core_adapter.py 51 17 coverage-67%
voicevox_engine/core_initializer.py 59 30 coverage-49%
voicevox_engine/core_wrapper.py 224 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 26 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 33 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 163 9 coverage-94%
voicevox_engine/morphing.py 71 46 coverage-35%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 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/init.py 3 0 coverage-100%
voicevox_engine/tts_pipeline/acoustic_feature_extractor.py 26 0 coverage-100%
voicevox_engine/tts_pipeline/kana_converter.py 86 1 coverage-99%
voicevox_engine/tts_pipeline/mora_list.py 4 0 coverage-100%
voicevox_engine/tts_pipeline/text_analyzer.py 132 6 coverage-95%
voicevox_engine/tts_pipeline/tts_engine.py 187 8 coverage-96%
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 2258 725 coverage-68%

Copy link
Member

@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.

ほぼLGTMです!!

関数渡しでDI可能にするのか、importする対象をmockで置き換えるのか、どっちが良いか難しいかもと思いました!
(基本的に関数渡してDI可能にする形が良さそう)

test/test_text_analyzer.py Outdated Show resolved Hide resolved
test/test_text_analyzer.py Outdated Show resolved Hide resolved
test/test_acoustic_feature_extractor.py Show resolved Hide resolved
Copy link
Member

@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.

LGTM!!

@Hiroshiba Hiroshiba merged commit 5d61144 into VOICEVOX:master Jan 3, 2024
3 checks passed
@tarepan tarepan deleted the add/text_analysis_test branch January 5, 2024 06:45
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.

2 participants