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

Session 8: Unit Test #7

Merged
merged 52 commits into from
Feb 28, 2024
Merged

Session 8: Unit Test #7

merged 52 commits into from
Feb 28, 2024

Conversation

kantacky
Copy link
Owner

@kantacky kantacky commented Feb 19, 2024

Session 8: Unit Test

Updates

  • テスト用にForecastViewModelMockを実装
  • テストターゲットを追加
  • ForecastViewModelのテストケースを実装
  • ViewInspectorの導入
  • ForecastViewのテストケースを実装
  • swift-dependenciesを導入
  • YumemiWeatherClientを実装
  • SwiftLintの導入

Issues

  • テストの書き方は正しいのか?
  • ViewInspectorを追加するとテストビルドの際にエラーになる
    スクリーンショット 2024-02-19 15 21 40

@mitsuharu
Copy link

CoreAudioTypes に関して Xcode の不具合の可能性がありますね。
https://developer.apple.com/forums/thread/736152

あと、ViewInspectorは単体テストでSwiftUIのテストを行うもので、UIテストではない認識です。
いまUITestsの方で実装されているので、CoreAudioTypesの問題が発生したのかなと思いました。
https://zenn.dev/ikeh1024/articles/0cebdc08c88222

@kantacky
Copy link
Owner Author

CoreAudioTypes に関して Xcode の不具合の可能性がありますね。 https://developer.apple.com/forums/thread/736152

あと、ViewInspectorは単体テストでSwiftUIのテストを行うもので、UIテストではない認識です。 いまUITestsの方で実装されているので、CoreAudioTypesの問題が発生したのかなと思いました。 https://zenn.dev/ikeh1024/articles/0cebdc08c88222

なるほど!
初めてのことなので認識が間違っていました...
調べてみます、ありがとうございます!

Copy link

@yusuga yusuga left a comment

Choose a reason for hiding this comment

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

今朝ちらっと話した DI について早速反映しててよいですねー👏 気になったところをコメントしました!

YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/YumemiWeatherClient.swift Outdated Show resolved Hide resolved
YumemiTrainingTests/ForecastViewModelTests.swift Outdated Show resolved Hide resolved
YumemiTrainingTests/ForecastViewModelTests.swift Outdated Show resolved Hide resolved
YumemiTrainingTests/ForecastViewModelTests.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
@kantacky kantacky mentioned this pull request Feb 21, 2024
@usami-k usami-k removed the request for review from a team February 21, 2024 06:34
@kantacky kantacky mentioned this pull request Feb 22, 2024
1 task
@kantacky kantacky force-pushed the session/7 branch 3 times, most recently from fd9505d to 7431a83 Compare February 22, 2024 04:11
Copy link

@yusuga yusuga left a comment

Choose a reason for hiding this comment

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

次は結構細かい部分で気になったところにコメントしました!

YumemiTraining/DateFormatter+.swift Outdated Show resolved Hide resolved
YumemiTraining/ForecastViewModel.swift Outdated Show resolved Hide resolved
YumemiTraining/Weather.swift Outdated Show resolved Hide resolved
YumemiTraining/NewView.swift Show resolved Hide resolved
YumemiTraining/TemeratureView.swift Outdated Show resolved Hide resolved
YumemiTraining/YumemiWeatherError+.swift Outdated Show resolved Hide resolved
Comment on lines 15 to 17
guard let now = DateFormatter.iso8601Full.date(from: "2020-04-01T12:00:00+09:00") else {
return
}
Copy link

Choose a reason for hiding this comment

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

must

return してしまうと、もしも日付文字列のタイプミスがあった場合に正しくテストができなくリスクがあるため避けた方が良いです。
明確なリスクとしては、もしも iso8601Full 内のフォーマットが変わった場合にテストが正しく実行できなくなってしまいます。そのため、最初の実装時にブレークポイントなどで手動でテストが実行されることを確認してたとしても、構造上防ぐようにしたいです。

対応案1: 強制アンラップ

まず最初に思いつくのは! ですね。これはもしもパースできなかったらテストが失敗します。ただし、強制アンラップが失敗したというランタイムエラーがでるだけなので、テストの失敗ログとしてはもうちょっと正しいものにしたいです。

let now = DateFormatter.iso8601Full.date(from: "2020-04-01T12:00:00+09:00")!

対応案2: テストを fail させる

次に XCTFail です。これはテストを失敗させられるかつ、任意のログを残せるので案1より良いと思います。

guard let now = try DateFormatter.iso8601Full.date(from: "2020-04-01T12:00:00+09:00") else {
  XCTFail("失敗した具体的な理由を書く")
}

問題

そこで問題!案2も実はベストプラクティスではなく、より良い方法がありますので探してみてください!

  • ヒント1: XCTest framework がオプショナル型に関する関数をいくつかもっています
  • ヒント2: XCTAssertNil と XCTAssertNotNil ではないです

Copy link
Owner Author

Choose a reason for hiding this comment

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

これですか...??
826065b

Copy link
Owner Author

Choose a reason for hiding this comment

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

7765252
WeatherRequest.encode()のJSONEncoder().dateEncodingStrategyを.iso8601にすると、エンコード結果が、2020-04-01T12:00:00+09:00ではなく、2020-04-01T03:00:00Zとなってしまいます。
APIとのやりとりはこれでも問題ないみたいなのですが、テストのこの部分だけを変えるのも納得いかないので、もし何か方法があれば教えていただきたいです...

Copy link

Choose a reason for hiding this comment

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

これですか...??
826065bef770a3

正解です!🙆‍♂️
こんな感じでちょっとずつベストな書き方ってなんだろうって調べていくのが成長につながると思います!

Copy link

Choose a reason for hiding this comment

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

WeatherRequest.encode()のJSONEncoder().dateEncodingStrategyを.iso8601にすると、エンコード結果が、2020-04-01T12:00:00+09:00ではなく、2020-04-01T03:00:00Zとなってしまいます。

+09:00 の JST でテストしたいのであれば、TimeZone を明示的に指定する方法を探してみてください!

Date 周りは結構難しいのですが、以下のような説明になります。

  • Date は特定の日時を表している型で、内部表現は unixtime のようなものを保持しているだけ。そのため 2020-04-01T12:00:00+09:002020-04-01T03:00:00Z は同じ Date 型の値になる。
  • DateFormatter / ISO8601DateFormatter は Date 型を人間が読みやすい文字列のフォーマットに変換(Date to String)またはparse(String to Date)するためのもの。そのため Locale や Timezone の指定がある。
    • Locale を指定するとどこの国の日付の読み方にするかを指定できる。例えば日本は年月日を表現するときは 年/月/日 の順に書くが、アメリカだと 月/日/年 の順になる。そういった表現も DateFormatter が対応してくれている。
    • TimeZone は時差のことで基準となる UTC(≒ GMT) を +00:00 または Z として、そこから + or - のどちらにずれているかを表現している。
    • Locale と TimeZone は Date 型は意識していないことが重要。あくまでも DateFormatter を通して人間が読む時の文字列表現をどうするかという話なだけ。
  • ここら辺の話は 【Swift】Dateの王道 【日付】 が結構詳しいです。
    • ただし、今は iOS 15 からサポートされた Date.FormatStyle を優先して使います。DateFormatter は実は生成コストがめちゃくちゃ高いのでキャッシュしたりする必要があるのですが、 Date.FormatStyle はその問題を解決しています。

Copy link
Owner Author

Choose a reason for hiding this comment

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

おそらく、.dateEncodingStrategyを.iso8601にするとGMTになってしまうので、
DateFormatterを作ってtimeZoneを指定したものを.formatted(DateFormatter)に渡してエンコードするということですよね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

acbd4d8
Encoder/Decoderを切り出して、責務を分離しました。
YumemiWeatherClientがEncoderとDecoderに依存する形で使用しています。

Copy link

@yusuga yusuga left a comment

Choose a reason for hiding this comment

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

LGTM です!

@kantacky kantacky merged commit a2e2629 into main Feb 28, 2024
@kantacky kantacky deleted the session/8 branch February 28, 2024 07:54
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.

None yet

3 participants