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

C APIのテストにvoicevox_core.hを使う #774

Merged
merged 8 commits into from
Apr 30, 2024

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Apr 3, 2024

内容

#488 で言った次のことをします。

bindgenの生成物はcrates/test_utilの$OUT_DIR下に作るようにしました。compatible_engineに関しては、COREの0.13を参考にcompatible_engine.hを書きました。

関連 Issue

その他

Comment on lines +44 to +68
for result_code in [
c_api::VoicevoxResultCode_VOICEVOX_RESULT_OK,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_NOT_LOADED_OPENJTALK_DICT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_GET_SUPPORTED_DEVICES_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_GPU_SUPPORT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_STYLE_NOT_FOUND_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_MODEL_NOT_FOUND_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INFERENCE_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_EXTRACT_FULL_CONTEXT_LABEL_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_UTF8_INPUT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_PARSE_KANA_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_AUDIO_QUERY_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_ACCENT_PHRASE_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_OPEN_ZIP_FILE_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_READ_ZIP_ENTRY_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_MODEL_ALREADY_LOADED_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_STYLE_ALREADY_LOADED_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_MODEL_DATA_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_LOAD_USER_DICT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_SAVE_USER_DICT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_USER_DICT_WORD_NOT_FOUND_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_USE_USER_DICT_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_USER_DICT_WORD_ERROR,
c_api::VoicevoxResultCode_VOICEVOX_RESULT_INVALID_UUID_ERROR,
] {
Copy link
Member Author

@qryxip qryxip Apr 4, 2024

Choose a reason for hiding this comment

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

ResultCode一覧をC API実装側から引っ張って来れていないのは前から。

今後エラーを追加するときにこっちの更新を忘れるかもしれませんが、網羅すべきなテストでもないのでいっそのことOKERROR一つに限定してもいいかもしれません。

Copy link
Member Author

Choose a reason for hiding this comment

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

いやbindgen側にstrumを差し込めるかも...?

Copy link
Member Author

@qryxip qryxip Apr 4, 2024

Choose a reason for hiding this comment

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

いやbindgen側にstrumを差し込めるかも...?

あーこれ、rustified_enumにする時点で厳しい… #[derive(strum::EnumIter)]を差し込むだけならbindgenの生成結果の上からsynで加工しようと思ってたのですが、rustified_enumの面倒を見るのは手間がかかりすぎる… (できなくはないけど、メンテするコード量的に)

Copy link
Member

Choose a reason for hiding this comment

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

よくわかってないですが、なんとかできそうだけどコピペのが手っ取り早そうな雰囲気を感じました!

@qryxip
Copy link
Member Author

qryxip commented Apr 10, 2024

なんか嫌な感じの落ち方してる…

@qryxip
Copy link
Member Author

qryxip commented Apr 10, 2024

よく見たら単にbindgen単体がコケているっぽい。ちゃんと調べずに使ったのが悪かったのかもしれない。

@qryxip
Copy link
Member Author

qryxip commented Apr 10, 2024

よく見たら単にbindgen単体がコケているっぽい。ちゃんと調べずに使ったのが悪かったのかもしれない。

いやそんなことないな。確かに落ちているのはbindgenだけど、何もしてないのに落ちるようになった。

…ということで試しに依存グラフ上のclang-sys (bindgenが依存)を1.4.0から1.6.0に引き上げてみたらなんか通るようになりました。
30ad64e (#774)

bindgenのChangelog
clang-sysのChangelog

考えられる可能性としては、GHAのイメージ更新があってそれとclang-sys v1.4.0の挙動が悪い方向に噛み合ってしまった感じ?

Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

LGTMです。(symbols.rsの更新要らなくなったの嬉し過ぎる)

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 e3cb7be into VOICEVOX:main Apr 30, 2024
36 checks passed
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.

3 participants