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のE2Eテストを作る #425

Merged
merged 79 commits into from
Apr 8, 2023
Merged

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Feb 5, 2023

内容

  • /crates/voicevox_core/src/test_util.rsを/crates/test_utilに分離する
  • /crates/test_util/src/bin/run-cdylib.rsを書き、 assert_cmdでDLLのE2Eテストをする機構を用意する
  • DLLのE2Eテストを書く

関連 Issue

Resolves #424.

その他

@qryxip
Copy link
Member Author

qryxip commented Feb 5, 2023

よく考えたらこれだとE2Eテストの内容がtest_utils側に来てしまいますね。
bin用クレートは用意しないでrusty-fork風にどうにかしてtestバイナリ自体を利用できないか考えた方がいいかも。

@qryxip
Copy link
Member Author

qryxip commented Feb 6, 2023

あ、わかった単純にharness外してバイナリを再帰的に呼べばいいですね。多分。

@qryxip
Copy link
Member Author

qryxip commented Feb 6, 2023

いやharnessが無くなるとそれはそれで面倒が生じる、と思ったのですが、代替harnessとしてlibtest-mimicというお誂え向きなものがありますね。これ使うのがいいかも。

参考: Custom test harnesses - cargo-nextest

@qryxip
Copy link
Member Author

qryxip commented Feb 6, 2023

テスト機構はとりあえずできました。

const char* voicevox_get_version(void)に対するテストが以下のように書けます。

use std::ffi::CStr;

use assert_cmd::assert::Assert;

use crate::Symbols;

pub(crate) unsafe fn exec(
    Symbols {
        voicevox_get_version,
    }: Symbols<'_>,
) -> anyhow::Result<()> {
    let version = voicevox_get_version();
    let version = CStr::from_ptr(version).to_str()?;
    println!("Version: {version:?}");
    Ok(())
}

pub(crate) fn assert(assert: Assert) {
    assert.success().stdout("Version: \"0.0.0\"\n").stderr("");
}

(追記) よく考えたらexec側でassert!すればいいですね。

テストが成功した場合:

image

テストが失敗した場合:

image

@qryxip
Copy link
Member Author

qryxip commented Feb 8, 2023

snapshots.tomlというファイルにAudioQueryやwavのSHA-256を書き、それをチェックするという方針にしました。
OSS版モデルと0.14.1のモデルの両方をチェックしてしまうことを考えています。
暗号化があるから製品版モデルだけじゃ駄目でしたね…DLLごと持ってきたとしても何に対するテストなのかという話になってしまう。

[metas]
metas-json = "675be97048d261b06ae5fe75a2614ba253422abdcc9eb90ad4a073875af0f57e"
use std::ffi::CStr;

use assert_cmd::assert::{Assert, AssertResult};
use serde::Deserialize;

use crate::Symbols;

use super::SNAPSHOTS;

#[derive(Deserialize)]
#[serde(rename_all = "kebab-case")]
pub(super) struct Snapshots {
    metas_json: String,
}

pub(crate) unsafe fn exec(Symbols { metas, .. }: Symbols<'_>) -> anyhow::Result<()> {
    let metas_json = metas();
    let metas_json = CStr::from_ptr(metas_json).to_str()?;
    std::assert_eq!(SNAPSHOTS.metas.metas_json, super::sha256(metas_json));
    Ok(())
}

pub(crate) fn assert_output(assert: Assert) -> AssertResult {
    assert.try_success()?.try_stdout("")?.try_stderr("")
}

@qryxip
Copy link
Member Author

qryxip commented Feb 8, 2023

あーautocrlf!!!

https://github.com/VOICEVOX/voicevox_core/actions/runs/4125239867

@qryxip
Copy link
Member Author

qryxip commented Feb 10, 2023

tracing-subscriberのタイムスタンプのマスクもできたし、あとはyukarin_sa_forwarddecode_forwardをやれば形にはなりそう。

@qryxip
Copy link
Member Author

qryxip commented Feb 10, 2023

"It is not used by any node and should be removed from the model"がActions環境だけ出てない...???

https://github.com/VOICEVOX/voicevox_core/actions/runs/4145617823/jobs/7170233703

@qryxip
Copy link
Member Author

qryxip commented Feb 10, 2023

あ! #423 があるから"It is not used by any node and should be removed from the model"が無いのが正しいんでした。
git pullを忘れて入ってませんでした。

@qryxip
Copy link
Member Author

qryxip commented Mar 9, 2023

そういえば今のRustならこんなこともできますね。その発想はありませんでした。

mod initialize前にモデルを読み込むとエラーになるテスト;
mod エンジンを起動してyukarin_sとyukarin_saとdecodeの推論を行う; // '・'や,や'、'は利用不可
// これもOK
#[path = "./testcases/エンジンを起動してyukarin_s・yukarin_sa・decodeの推論を行う.rs"]
mod _0; 
#[path = "./testcases/initialize前にモデルを読み込むとエラーになるテスト.rs"]
mod _1;
// あるいはこれでもOK

const _: () = {
    #[path = "./testcases/エンジンを起動してyukarin_s・yukarin_sa・decodeの推論を行う.rs"]
    mod _testcase;
};

const _: () = {
    #[path = "./testcases/initialize前にモデルを読み込むとエラーになるテスト.rs"]
    mod _testcase;
};

(追記) いやrust-analyzerが混乱しそうではありますが...

@qryxip
Copy link
Member Author

qryxip commented Mar 9, 2023

あと、他の人が迷子にならないように、assert_cdylib.execsnapshots.sectionSymbols辺りの一般的な単語を使っているクラス名のものは1行くらいで良いので説明コメントがあれば、それだけでもかなり読みやすくなる気がしました!!

入れてみました。

image

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!!!!!!

ありがとうございました!!

コメントにsuggestコメントしてみました!!(認識合わせも兼ねて)

crates/voicevox_core_c_api/tests/e2e/assert_cdylib.rs Outdated Show resolved Hide resolved
crates/voicevox_core_c_api/tests/e2e/assert_cdylib.rs Outdated Show resolved Hide resolved
crates/voicevox_core_c_api/tests/e2e/snapshots.rs Outdated Show resolved Hide resolved
@Hiroshiba
Copy link
Member

あと1名Rustに明るい方にレビューをお願いできるととても心強いです・・・!!

@PickledChair さん、 @qwerty2501 さんお願いできますか 🙇
1000行ほどあるのでちょっとだけ共有をば。

  • e2eテストの入り口はe2e/main.rs
  • e2e/testcasesに各テストケースを書く
  • stdoutログなどのテストも含むため各テストは別プロセスで起動する
  • 各テストが実行されるか・ログが意図通りかがテストされる

@qryxip
Copy link
Member Author

qryxip commented Mar 14, 2023

#370 見てたらサニタイザを挟みたくなってきました…
ONNX RuntimeやOpen JTalkとかのバインディング及び本体自体の課題も検出できるはず。
LLVMのやつはNightly Rustを使う必要があるので面倒ですが、Valgrindならバイナリひとつ持ってきて挟むだけでいいはず。

@qryxip
Copy link
Member Author

qryxip commented Mar 18, 2023

LLVMのやつ(-Z sanitizer)の方がいい気がしてきました。単体テストも対象にできますし。
Nightly Rustを管理する必要がありますが、Nightlyなら-Z avoid-dev-depsとかcargo-udepsとか使えるし悪くないかと。
その場合このPRじゃなくて別PRでの導入ですね。

@qryxip
Copy link
Member Author

qryxip commented Mar 24, 2023

VOICEVOX/onnxruntime-rs#16によって

こんな感じ
{timestamp}  INFO onnxruntime::onnxruntime: "Creating and using per session threadpools since use_per_session_threads_ is true"
{timestamp}  INFO onnxruntime::onnxruntime: "Dynamic block base set to 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Initializing session."
{timestamp}  INFO onnxruntime::onnxruntime: "Adding default CPU execution provider."
{timestamp}  INFO onnxruntime::onnxruntime: "Creating BFCArena for Cpu with following configs: initial_chunk_size_bytes: 1048576 max_dead_bytes_per_chunk: 134217728 initial_growth_chunk_size_bytes: 2097152 memory limit: 18446744073709551615 arena_extend_strategy: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Total fused reshape node count: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Saving initialized tensors."
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:0 (requested) num_bytes: 24 (actual) rounded_bytes:256"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 1048576 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 1048576"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd97feff040 to 0x7fd97ffff040"
{timestamp}  INFO onnxruntime::onnxruntime: "Done saving initialized tensors"
{timestamp}  INFO onnxruntime::onnxruntime: "Session successfully initialized."
{timestamp}  INFO onnxruntime::onnxruntime: "Creating and using per session threadpools since use_per_session_threads_ is true"
{timestamp}  INFO onnxruntime::onnxruntime: "Dynamic block base set to 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Initializing session."
{timestamp}  INFO onnxruntime::onnxruntime: "Adding default CPU execution provider."
{timestamp}  INFO onnxruntime::onnxruntime: "Creating BFCArena for Cpu with following configs: initial_chunk_size_bytes: 1048576 max_dead_bytes_per_chunk: 134217728 initial_growth_chunk_size_bytes: 2097152 memory limit: 18446744073709551615 arena_extend_strategy: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Total fused reshape node count: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Total fused reshape node count: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Saving initialized tensors."
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:0 (requested) num_bytes: 8 (actual) rounded_bytes:256"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 1048576 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 1048576"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd97d5f9040 to 0x7fd97d6f9040"
{timestamp}  INFO onnxruntime::onnxruntime: "Done saving initialized tensors"
{timestamp}  INFO onnxruntime::onnxruntime: "Saving initialized tensors."
{timestamp}  INFO onnxruntime::onnxruntime: "Done saving initialized tensors"
{timestamp}  INFO onnxruntime::onnxruntime: "Subgraph input with name cond is not used by any node."
{timestamp}  INFO onnxruntime::onnxruntime: "Session successfully initialized."
{timestamp}  INFO onnxruntime::onnxruntime: "Creating and using per session threadpools since use_per_session_threads_ is true"
{timestamp}  INFO onnxruntime::onnxruntime: "Dynamic block base set to 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Initializing session."
{timestamp}  INFO onnxruntime::onnxruntime: "Adding default CPU execution provider."
{timestamp}  INFO onnxruntime::onnxruntime: "Creating BFCArena for Cpu with following configs: initial_chunk_size_bytes: 1048576 max_dead_bytes_per_chunk: 134217728 initial_growth_chunk_size_bytes: 2097152 memory limit: 18446744073709551615 arena_extend_strategy: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Total fused reshape node count: 0"
{timestamp}  INFO onnxruntime::onnxruntime: "Saving initialized tensors."
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:0 (requested) num_bytes: 8 (actual) rounded_bytes:256"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 1048576 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 1048576"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd955d7d040 to 0x7fd955e7d040"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:0 (requested) num_bytes: 192 (actual) rounded_bytes:256"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 2097152 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 3145728"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd955b7c040 to 0x7fd955d7c040"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:13 (requested) num_bytes: 2883584 (actual) rounded_bytes:2883584"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 4194304 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 7340032"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd95577b040 to 0x7fd955b7b040"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:0 (requested) num_bytes: 24 (actual) rounded_bytes:256"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 8388608 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 15728640"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd954f7a040 to 0x7fd95577a040"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:15 (requested) num_bytes: 8388608 (actual) rounded_bytes:8388608"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 16777216 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 32505856"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd953f79040 to 0x7fd954f79040"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:13 (requested) num_bytes: 2883584 (actual) rounded_bytes:2883584"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 33554432 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 66060288"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd951f78040 to 0x7fd953f78040"
{timestamp}  INFO onnxruntime::onnxruntime: "Done saving initialized tensors"
{timestamp}  INFO onnxruntime::onnxruntime: "Extending BFCArena for Cpu. bin_num:15 (requested) num_bytes: 8388608 (actual) rounded_bytes:8388608"
{timestamp}  INFO onnxruntime::onnxruntime: "Extended allocation by 67108864 bytes."
{timestamp}  INFO onnxruntime::onnxruntime: "Total allocated bytes: 133169152"
{timestamp}  INFO onnxruntime::onnxruntime: "Allocated memory at 0x7fd94df77040 to 0x7fd951f77040"
{timestamp}  INFO onnxruntime::onnxruntime: "Session successfully initialized."
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"
{timestamp}  INFO onnxruntime::onnxruntime: "Begin execution"

のメッセージが出るようになったのですが、releaseプロファイルでのビルドなら出なくなるため--releaseのDLLをテストしてしまうことにしました。
"E2E"なのだからこっちの方が実態に即しているはず。

Copy link
Member

@PickledChair PickledChair left a comment

Choose a reason for hiding this comment

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

すみません、かなりお待たせしてしまいましたが、LGTM です!(自分の知らない有用な crate やアイディアが豊富だったので、全体を理解するのに時間がかかってしまいました……)

テストの追加もやりやすそうになっており、ありがたいです。

@Hiroshiba
Copy link
Member

マージします!!

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.

C APIのE2Eテストを作る
3 participants