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

Add initial test cases #38

Merged
merged 7 commits into from
Mar 18, 2024
Merged

Add initial test cases #38

merged 7 commits into from
Mar 18, 2024

Conversation

kvvzr
Copy link
Contributor

@kvvzr kvvzr commented Mar 16, 2024

Added first tests using TestStore.

Ref: #28

Copy link
Member

@d-date d-date left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! And I ask you to fix some points:

  • Why you back to KeyPath style: @DependencyClient will generate testValue().
  • Dummy data should be under your tests if you use only for testing.

MyLibrary/Sources/DataClient/Client.swift Outdated Show resolved Hide resolved
MyLibrary/Sources/DataClient/Client.swift Outdated Show resolved Hide resolved
MyLibrary/Sources/DataClient/Client.swift Outdated Show resolved Hide resolved
@kvvzr
Copy link
Contributor Author

kvvzr commented Mar 17, 2024

I have fixed the pointed out issues.

One point of concern is that I named it 'mock' based on the sample code from TCA, but thinking about it, 'dummy' might be a better choice. What do you think?

Comment on lines 45 to 46
static public let testValue = Self()

Copy link
Member

Choose a reason for hiding this comment

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

This is also macro's roll

@@ -38,6 +38,7 @@ public struct Schedule {
case path(StackAction<Path.State, Path.Action>)
case destination(PresentationAction<Destination.Action>)
case view(View)
case fetchResponse(Result<(Conference, Conference, Conference), Error>)
Copy link
Member

Choose a reason for hiding this comment

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

Might be better with creating struct.

Suggested change
case fetchResponse(Result<(Conference, Conference, Conference), Error>)
case fetchResponse(Result<SchedulesResponse, Error>)

The struct may be like below:

public struct SchedulesResponse: Equatable {
  var day1: Conference
  var day2: Conference
  var workshop: Conference
}

MyLibrary/Sources/ScheduleFeature/Schedule.swift Outdated Show resolved Hide resolved
@d-date
Copy link
Member

d-date commented Mar 17, 2024

One point of concern is that I named it 'mock' based on the sample code from TCA, but thinking about it, 'dummy' might be a better choice. What do you think?

Either is OK in this test.

Copy link
Member

@d-date d-date left a comment

Choose a reason for hiding this comment

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

Thank you for adding test! Now we're time to run it on CI!

@d-date d-date merged commit eef1a4d into tryswift:main Mar 18, 2024
@kvvzr
Copy link
Contributor Author

kvvzr commented Mar 18, 2024

Thank you for your review!

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