US1955755: add refactored tests under new card config integration tes… #230
Conversation
| return colorInt | ||
| } | ||
|
|
||
| protected inline fun wait2(maxWaitTimeInMillis: Int = 1000, assertions: () -> Unit) { |
There was a problem hiding this comment.
Given that this is asserting can we rename it so it conveys that?
There was a problem hiding this comment.
This was inherited from the class that already exists in master, I will rename to waitForAssertion()
| enabledStateIs(pan = true, cvc = true, expiryDate = true, submitButton = false) | ||
| cardDetailsAre(pan = "", cvc = "", expiryDate = "") | ||
| hasNoBrand() | ||
| paymentsCvcSessionCheckedState(checked = false) |
There was a problem hiding this comment.
paymentsCvcSessionCheckedState() : let's stick to using the terminology paymentsCvcSwitch and also let's try to keep the naming aligned with other functions that assert things/states and are suffixed with Is or Are, e.g. paymentsCvcSwitchIs(checked/on = false) ?
There was a problem hiding this comment.
Happy to refactor the method name, it was inherited from the class that already exists in master
| onView(withId(android.R.id.button1)).perform(click()) | ||
| } | ||
|
|
||
| fun isInErrorState(pan: String? = null, cvc: String? = null, expiryDate: String? = null) = apply { |
There was a problem hiding this comment.
I don't see anything in the body of the function that checks that something is in error state? Are we missing something or does the function need renaming?
There was a problem hiding this comment.
This is inherited from the class that already exists in master to avoid any regression
.../java/com/worldpay/access/checkout/sample/card/standard/testutil/NewCardFragmentTestUtils.kt
Show resolved
Hide resolved
| if (pan != null) wait2 { onView(withText(expiryDate)).check(matches(isDisplayed())) } | ||
| } | ||
|
|
||
| fun validationStateIs( |
There was a problem hiding this comment.
Can we put the arguments on the same line, like other functions, so we are consistent in our code style?
There was a problem hiding this comment.
Yeah sure I will make that change
| import org.junit.Test | ||
|
|
||
| class NewDiscoveryIntegrationTest { | ||
| // WIP |
| } | ||
|
|
||
| @Test | ||
| fun shouldRetryDiscoveryAndReturnSuccessfulResponse_whenDiscoveryFailsFirstTime_cardFlow() { |
There was a problem hiding this comment.
successfulResponse is misleading because it seems to pertain to the discovery. Can we rename AndReturnSuccessfulResponse maybe AndSuccessfullyCreatesSession?
| } | ||
|
|
||
| @Test | ||
| fun shouldRetryDiscoveryAndReturnSuccessfulResponse_whenDiscoveryFailsFirstTime_cvcFlow() { |
| } | ||
|
|
||
| @Test | ||
| fun givenCardConfigCallFails_validKnownBrandCardDetails_returnsSuccessfulResponse() { |
There was a problem hiding this comment.
Same comment as in class above
There was a problem hiding this comment.
This method name was inherited from the class that already exists in master
.../src/androidTestMock/java/com/worldpay/access/checkout/sample/cvc/NewCvcFragmentTestUtils.kt
Show resolved
Hide resolved
|
Adding screenshot of a local execution to show newly added tests pass in suite 
|
What
CardConfigurationIntegrationTestin new classNewCardConfigurationIntegrationTestto not depend on Activity context as it goes against Google's guidance for UI testingreturn thisas it returns a static reference to that Java class which made refactoring more difficult when amending class names. Scope functions help negate thisWhy
We should strive to align the codebase with Google's guidance to ensure we can minimise pain when updating dependencies and to avoid pain when refactoring the code base in future
Rally Link