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

[POS] Custom payment UI — Part 5 | Unit tests clean up #12943

Merged
merged 92 commits into from
Nov 28, 2024

Conversation

samiuelson
Copy link
Collaborator

@samiuelson samiuelson commented Nov 18, 2024

#12943

Description

The intent of This PR is to clean up unit tests of CardReaderPaymentControllerTest and CardReaderPaymentViewModelTest.

This is part 5 of 5 PRs refactoring the Payment flow:

  1. [POS] Custom payment UI — Part 1 | Separating payment collection management from Android Framework
  2. [POS] Custom payment UI — Part 2 | Separating CardReaderPaymentController's events from MultiLiveEvent
  3. [POS] Custom Payment UI – Part 3 | Emitting UI–agnostic payment states in CardReaderPaymentController
  4. [POS] Custom payment UI — Part 4 | Switch from viewStateData to paymentState
  5. [POS] Custom payment UI — Part 5 | Unit tests clean up

💡 This PR intends to merge changes to the feature/custom-payment-ui feature branch, that will be additionally tested and merged to trunk later.

Testing information

This PR is quite big, however, it modifies only two files: CardReaderPaymentControllerTest and CardReaderPaymentViewModelTest. It also does not introduce any new tests, only moves test methods around so that they are closer to the class they are testing.

  1. ViewState tests
    I copied all the unit test methods verifying CardReaderPaymentViewModel.viewStateData to CardReaderPaymentControllerTest and adapted them to test CardReaderPaymentController.paymentState instead. I left original tests as well because they are testing the CardReaderPaymentStateToViewStateMapper as a black box.
  2. Tests not testing ViewState were moved to CardReaderPaymentControllerTest (and removed from CardReaderPaymentViewModelTest)

The tests that have been performed

I verified that CardReaderPaymentViewModelTest does not contain test methods that are more suitable for the controller.

Testing

I recommend checking this PR as a whole

  1. Review the CardReaderPaymentViewModelTest and look if there is a class that could be moved or copied/adapted to the CardReaderPaymentController class.
  2. Verify that unit tests and other CI checks are passing

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@samiuelson samiuelson added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 18, 2024
`given collect payment shown, when RETRY message received, then collect payment hint updated`
`given collect payment shown, when INSERT_CARD received, then collect payment hint updated`
`given collect payment shown, when SWIPE_CARD received, then collect payment hint updated`
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 18, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commite9cd84d
Direct Downloadwoocommerce-wear-prototype-build-pr12943-e9cd84d.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 18, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commite9cd84d
Direct Downloadwoocommerce-prototype-build-pr12943-e9cd84d.apk

`given collect payment shown, when REMOVE_CARD received, then collect payment hint updated`
`given collect payment shown, when TRY_OTHER_CARD message received, then collect payment hint updated`
`given collect payment shown, when CARD_REMOVED_TOO_EARLY message received, then collect payment hint updated`
`given collect payment shown, when TRY_OTHER_READ message received, then collect payment hint updated`
`given collect payment shown, when MULTIPLE_CARDS_DETECTED received, then collect payment hint updated`
`given fetching order fails, when payment screen shown, then ExternalReaderFailedPayment state is shown`
`given fetching order fails and tpp, when payment screen shown, then BuiltInReaderFailedPayment state is shown`
`when fetching order fails, then event tracked`
`given fetching order succeeds, when payment screen shown, then order currency stored `
`when payment screen shown, then loading data state is emitted`
`when payment not collectable, then error event emitted and flow terminated`
`when flow started, then correct payment description is propagated to CardReaderManager`
`when flow started, then correct statement descriptor is propagated to CardReaderManager`
`when initializing payment, then Loading state emitted`
`when collecting payment, then CollectingPayment state emitted`
`given built in reader,when collecting payment, then ui CollectingPayment state emitted`
`when processing payment, then ProcessingPayment state emitted`
`given built in reader, when processing payment, then ProcessingPayment state emitted`
`when processing payment completed with card present, then tracking keeper stores payment type`
`given billing email empty and built in, when user clicks on print receipt button, then event tracked`
`given billing email not empty and external, when user clicks on print receipt button, then event tracked`
`given billing email not empty and built in, when user clicks on print receipt button, then event tracked`
`given get receipt url fails, when user clicks on print receipt button, then error event emitted`
`given get receipt url succeeds, when user clicks on print receipt button, then PrintReceipt emitted`
`when OS accepts the print request, then print success event tracked`

`when OS refuses the print request, then print failed event tracked`

`when manually cancels the print request, then print cancelled event tracked`
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:background="?attr/colorSurface"

Check warning

Code scanning / Android Lint

Overdraw: Painting regions more than once Warning

Possible overdraw: Root element paints background ?attr/colorSurface with a theme that also paints a background (inferred theme is @style/Theme.Woo.DayNight)
@@ -4372,4 +4381,5 @@
<string name="woo_shipping_labels_package_creation_add_package">Add Package</string>
<string name="woo_shipping_labels_package_creation_box_type">Box</string>
<string name="woo_shipping_labels_package_creation_envelope_type">Envelope</string>
<string name="email_not_registered_wpcom">Hmm, we can\'t find a WordPress.com account connected to this email address.</string>

Check warning

Code scanning / Android Lint

Unused resources Warning

The resource R.string.email_not_registered_wpcom appears to be unused
Base automatically changed from custom-payment-ui-4 to feature/custom-payment-ui November 25, 2024 13:13
@samiuelson samiuelson removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 25, 2024
@samiuelson samiuelson added this to the 21.3 milestone Nov 25, 2024
@samiuelson samiuelson added the feature: mobile payments Related to mobile payments / card present payments / Woo Payments. label Nov 25, 2024
@samiuelson samiuelson marked this pull request as ready for review November 25, 2024 20:02
@samiuelson samiuelson merged commit 4f8a3bc into feature/custom-payment-ui Nov 28, 2024
15 checks passed
@samiuelson samiuelson deleted the custom-payment-ui-5 branch November 28, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: mobile payments Related to mobile payments / card present payments / Woo Payments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants