Skip to content

fix tests with broken context mocks in android/player#774

Merged
tmarmer merged 3 commits intomainfrom
fix-android-tests
Dec 15, 2025
Merged

fix tests with broken context mocks in android/player#774
tmarmer merged 3 commits intomainfrom
fix-android-tests

Conversation

@tmarmer
Copy link
Copy Markdown
Contributor

@tmarmer tmarmer commented Dec 12, 2025

Issue: Some unit tests in android/player were not running because MockK throwing errors when it could not create a mock of android.content.Context. The errors were being swallowed by something and the test runner would treat each of these files as having no tests rather than causing a failure.

Fix:

  • Move broken tests to be instrumented tests under androidTest.
  • Migrate these tests from using junit5 to junit4
    • To support this, updated io.mockk:mockk to version 1.12.5. JUnit4 support was added in 1.12.4 (notes) so I updated to the latest non-breaking version I could find.
  • use the application context instead of a mock context
  • Fix any test failures after doing the above. These failures were
    • Some tests in AndroidPlayerTest checked the number of plugins. The list is now size 6 instead of 5
    • Some tests in SimpleAssetTest were failing with broken styles. Changed style used in these tests
    • HydrationScopeTest was failing the test test hydration scope is cancelled on flow end due to a race condition in the assertions. Added a waitForCondition to fix this.

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs

@tmarmer tmarmer requested review from a team as code owners December 12, 2025 20:42
@tmarmer tmarmer added the skip-release Preserve the current version when merged label Dec 12, 2025
Comment thread android/player/BUILD Outdated
Comment thread android/player/BUILD
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.77%. Comparing base (9ae91bc) to head (e5ccb1a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #774   +/-   ##
=======================================
  Coverage   85.77%   85.77%           
=======================================
  Files         504      504           
  Lines       22679    22679           
  Branches     2651     2651           
=======================================
  Hits        19454    19454           
  Misses       2896     2896           
  Partials      329      329           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread android/player/BUILD
]

# TODO: Update this to be updated automatically instead of maintaining a list manually
instrumented_test_classes = ['com.intuit.playerui.android.AndroidPlayerTest', 'com.intuit.playerui.android.AssetContextTest', 'com.intuit.playerui.android.AssetContextAndroidTest', 'com.intuit.playerui.android.renderer.NestedAssetTest', 'com.intuit.playerui.android.renderer.HydrationScopeTest', 'com.intuit.playerui.android.renderer.SimpleAssetTest', 'com.intuit.playerui.android.renderer.BrokenAssetTest', 'com.intuit.playerui.android.extensions.IntoKtTest', 'com.intuit.playerui.android.registry.RegistryPluginTest']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need to do this in order to get all these tests picked up

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah — there is logic here for automatically compiling this list, but it picks up every file. Ideally, we can expose that logic to let us filter out files (like not ending in Test) to build up the list here. We could also put the filter in the rule, but that's pretty opinionated.

Copy link
Copy Markdown
Contributor Author

@tmarmer tmarmer Dec 12, 2025

Choose a reason for hiding this comment

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

Yeah, if I don't add them manually, it picks up every .kt file as a test and any of the util / non-test files end up creating test targets that just throw errors.

val player = AndroidPlayer()

assertEquals(5, player.plugins.size)
assertEquals(6, player.plugins.size)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when did the plugin numbers change? did we add some new default plugin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like AsyncHydrationTrackerPlugin was added to buildDefaultPlugins of AndroidPlayer (code) about 5 months ago. I assume that's what made the difference here.

@tmarmer tmarmer merged commit ea25ef2 into main Dec 15, 2025
15 checks passed
@tmarmer tmarmer deleted the fix-android-tests branch December 15, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-release Preserve the current version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants