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

Holidayで自動テストの活用方法を検討し追加する #7

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

sus-taguchi-t
Copy link
Contributor

何の変更を加えましたか?

  • 自動テストを実装しました
  • 「最大人数を超えた場合はユーザーに通知してアバター選択画面に戻ること」を除いて Functional Test に応じたアサート文を入れました

何を確認しましたか?

  • ゴールデンパスのアサートがすべて通ることを確認しました
  • Failure Test および「最大人数を超えた場合はユーザーに通知してアバター選択画面に戻ること」でしか通らない箇所を除いてすべてのカバレッジが取得できることを確認しました

実装

  • 実行時の動きが分かるように、ログ(Error/Warn/Info/Debug)を出力していることを確認しました
  • 静的解析で問題が見つからないことを確認しました

テスト

テスト用のコード改変なので以下は対象外。

  • 変更影響がある全てのテストが成功することを確認しました
  • 変更影響があるソースのテストカバレッジが100%になることを確認しました

変更影響

  • GuideのSample Applicationに変更が反映されることを確認しました

レビュアーへのメッセージ

@@ -0,0 +1,38 @@
{
"name": "Extreal.SampleApp.Holiday.Tests",
Copy link
Contributor

Choose a reason for hiding this comment

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

Holiday.TestsにPerformanceTestと今回のテストをまとめた方がAssets直下のディレクトリが増えなくて、全体を理解しやすそうと感じました。


namespace Extreal.SampleApp.Holiday.Tests
{
public class GoldenPath
Copy link
Contributor

Choose a reason for hiding this comment

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

機能テストに合わせてるのであればそういうクラス名にした方が追跡しやすいと思います。

"com.unity.ide.visualstudio": "2.0.17",
"com.unity.ide.vscode": "1.2.5",
"com.unity.inputsystem": "1.5.0",
"com.unity.mobile.android-logcat": "1.3.2",
"com.unity.render-pipelines.universal": "12.1.10",
"com.unity.test-framework": "1.3.0",
"com.unity.testtools.codecoverage": "1.2.2",
"com.unity.testtools.codecoverage": "1.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

unityのcode coverage、1.2.3でやっとgeneric typeのバグ治ったので上げても大丈夫かもしれないです。
https://docs.unity3d.com/Packages/[email protected]/changelog/CHANGELOG.html

[UnityTest]
public IEnumerator GoldenPathTest() => UniTask.ToCoroutine(async () =>
{
// Loads application
Copy link
Contributor

Choose a reason for hiding this comment

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

コメントとLoggerで同じ内容を書いているので、コメントは削除してよいと思います。

Logger.LogInfo("Inputs player name");
var playerNameInputField = FindObjectOfTypeAndAssert<TMP_InputField>();

playerNameInputField.Select();
Copy link
Contributor

Choose a reason for hiding this comment

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

全体的にユーザー操作がイメージできるような抽象度になってないので、理解とメンテ負荷が高いように思います。


// Starts to download data and enters AvatarSelectionScreen
Logger.LogInfo("Starts to download data and enters AvatarSelectionScreen");
titleGoButton.onClick.Invoke();
Copy link
Contributor

Choose a reason for hiding this comment

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

今どのステージのどのシーンが対象なのか分かるような書き方にできないかな、と思いました。
さらに操作対象のステージやシーンのアサートも合った方がいいと思います。

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