-
Notifications
You must be signed in to change notification settings - Fork 241
Convert VenmoClientUnitTest to kotlin #1306
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
base: main
Are you sure you want to change the base?
Conversation
Venmo/src/test/java/com/braintreepayments/api/venmo/VenmoClientUnitTest.kt
Outdated
Show resolved
Hide resolved
Venmo/src/test/java/com/braintreepayments/api/venmo/MockVenmoApiBuilder.java
Show resolved
Hide resolved
Venmo/src/test/java/com/braintreepayments/api/venmo/MockkVenmoApiBuilder.kt
Outdated
Show resolved
Hide resolved
Venmo/src/test/java/com/braintreepayments/api/venmo/VenmoClientUnitTest.kt
Outdated
Show resolved
Hide resolved
sut = VenmoClient( | ||
braintreeClient, | ||
apiClient, | ||
venmoApi, | ||
sharedPrefsWriter, | ||
analyticsParamRepository, | ||
merchantRepository, | ||
venmoRepository, | ||
getReturnLinkTypeUseCase, | ||
getReturnLinkUseCase | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are your thoughts on creating the VenmoClient in the @before function? One benefit is that we won't need to update all unit tests if the dependencies end up changing. It also might not be possible if we need to test certain behavior in the VenmoClient's init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I was thinking about this. You mentioning it makes me think it's worth the added effort. I'll look into it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so there are like 13 iterations of VenmoClient parameters that are passed in the tests depending on what functionality is being tested. I think there might be a way to abstract away some of the parameters since braintreeClient
and venmoApi
are the only ones that change, but I think that might be more complicated than simply creating the client in the @before
Venmo/src/test/java/com/braintreepayments/api/venmo/VenmoClientUnitTest.kt
Outdated
Show resolved
Hide resolved
@@ -7,6 +7,7 @@ | |||
|
|||
import org.mockito.stubbing.Answer; | |||
|
|||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If deprecating do we need to call out in Changelog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great question! I figured that since it's only relevant to unit tests and isn't used anywhere it wasn't necessary to call it out in the CHANGELOG, but that was just a guess on best protocol. Do you all have any guidelines on CHANGELOG messages for tests?
Summary of changes
VenmoClientUnitTest
file from Java to KotlinMockVenmoApiBuilder
to Kotlin and deprecates the java versionMockkBraintreeClientBuilder
to support venmo test functionalityChecklist
Authors