Skip to content

[2/2] WCProductVariationModel to Room: unit tests adjustments #13982

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

Conversation

wzieba
Copy link
Contributor

@wzieba wzieba commented Apr 28, 2025

Description

This PR adjusts unit tests execution after migrating WCProductVariationModel to Room in #13972 .

Testing information

Please run the app and test different scenarios when using product variations. I don't have any specific scenario in mind, as this is a core change - this will affect all features related to variations.

Tip: you can set up a store with Jurassic Ninja and import the default set of products (https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/sample-data/sample_products.csv). One of the products has a few variations already to start with.

The tests that have been performed

Smoke tests of variations.

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.

wzieba added 4 commits April 25, 2025 16:29
2 tests are failing right now
…ven`

Room and WellSql have different defaults for sort. In this test, the order doesn't matter as all items have `menuOrder=0`
Don't use incorrect `$original.id` for description: instead, use simple string to avoid any confusion.

Close database on test end
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 28, 2025

📲 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
Commit3e6c29c
Direct Downloadwoocommerce-wear-prototype-build-pr13982-3e6c29c.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 28, 2025

📲 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
Commit3e6c29c
Direct Downloadwoocommerce-prototype-build-pr13982-3e6c29c.apk

wzieba added 2 commits April 28, 2025 16:36
…rn default values

Previously, functions like `VariationRepository#getProductVariationList` or `ProductDetailRepository#getCachedVariationCount` were *not* suspend. This allowed Mockito to return default values for both of the methods, respectively: empty list and 0.

After recent changed, both of this functions were refactored to suspend, which caused Mockito to return `null` for them (see: mockito/mockito-kotlin#342)

This comment brings back the previous behavior by mocking mentioned methods to return default values.
@wzieba wzieba changed the title [WIP] WCProductVariationModel to Room: unit tests adjustments [2/2] WCProductVariationModel to Room: unit tests adjustments Apr 30, 2025
@wzieba wzieba force-pushed the migrate_product_variation_to_room_tests branch from d42c2e3 to 3e6c29c Compare April 30, 2025 17:48
@wzieba wzieba marked this pull request as ready for review April 30, 2025 18:02
@wzieba wzieba requested a review from ParaskP7 April 30, 2025 18:02
@ParaskP7
Copy link
Contributor

ParaskP7 commented May 8, 2025

FYI: I am going to merge this PR into its feature branch, just like I'll first do with #13972, for the very same reasons I explained in this comment.

Cc @wzieba

Base automatically changed from migrate_product_variation_to_room to feature/product_variation_to_room May 8, 2025 11:20
@ParaskP7 ParaskP7 merged commit f40384f into feature/product_variation_to_room May 8, 2025
22 checks passed
@ParaskP7 ParaskP7 deleted the migrate_product_variation_to_room_tests branch May 8, 2025 11:21
@ParaskP7
Copy link
Contributor

ParaskP7 commented May 8, 2025

FYI: As I was testing this change and running unit tests locally (in CLI), using the ./gradlew testDebugUnitTest was consistently failing on 1087 test with the below (example) stack-trace on ByteBuddyAgent, similar to the Java 11 versus 17 problem we had with Mockito a while back:

    UploadStoreUnitTest > testNumberOfAutoUploadsAttemptsCounter FAILED
        java.lang.IllegalStateException at UploadStoreUnitTest.java:46
            Caused by: java.lang.IllegalStateException at PluginInitializer.java:56
                Caused by: java.lang.reflect.InvocationTargetException at NativeConstructorAccessorImpl.java:-2
                    Caused by: org.mockito.exceptions.base.MockitoInitializationException at InlineDelegateByteBuddyMockMaker.java:260
                        Caused by: java.lang.IllegalStateException at ByteBuddyAgent.java:706

The interesting thing is that tests run successfully when I split testDebugUnitTest into multiple modules, running them as:
- ./gradlew libs:fluxc:testDebugUnitTest
- ./gradlew libs:fluxc-plugin:testDebugUnitTest
- ./gradlew libs:fluxc-tests:testDebugUnitTest

What's more interesting, after me doing the above, even ./gradlew testDebugUnitTest start working as expected (I also used the --rerun-tasks flag to make sure this unit test task runs again).

Any idea on what might the problem be, did it happen on your side? Is it maybe related to testFixtures, I can't think of any other major change that could start causing this? 🤔

PS: Also, I had no trouble running those unit tests from within the IDE (AS).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants