Skip to content

test(DeckPicker): implement FAB visibility and expansion tests for DeckPickerFloatingActionMenu#20772

Open
MandeepT19 wants to merge 1 commit intoankidroid:mainfrom
MandeepT19:test/deck-picker-floating-action-menu
Open

test(DeckPicker): implement FAB visibility and expansion tests for DeckPickerFloatingActionMenu#20772
MandeepT19 wants to merge 1 commit intoankidroid:mainfrom
MandeepT19:test/deck-picker-floating-action-menu

Conversation

@MandeepT19
Copy link
Copy Markdown
Contributor

@MandeepT19 MandeepT19 commented Apr 18, 2026

Purpose / Description

Implements the unit tests requested by the @NeedsTest("reimplement: doubleTapAddsNote; singleTapTogglesFab") annotation on DeckPickerFloatingActionMenu.

Fixes

Part of #13283
@NeedsTest("reimplement: doubleTapAddsNote; singleTapTogglesFab") annotation in DeckPickerFloatingActionMenuTest

Approach

Identified that @Config(application = EmptyApplication::class) was preventing AnkiDroidApp.isInitialized from being true which caused showedActivityFailedScreen to exit onCreate early. Removing EmptyApplication from @Config fixed this. Added two Robolectric unit tests in /test and marked floatingActionMenu as internal with @VisibleForTesting in DeckPicker.kt as suggested.

showFloatingActionButtonMakesFabVisible verifies FAB is VISIBLE after showFloatingActionButton() is called
hideFloatingActionButtonMakesFabGone verifies FAB is GONE after hideFloatingActionButton() is called

Note: The doubleTapAddsNote is not covered here as there is no double tap test utility exists in the current test infrastructure and I am open to guidance if needed.

How Has This Been Tested?

Both tests pass via Robolectric on JVM without needing an emulator.

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link
Copy Markdown
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Prefer unit tests over Android tests

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Apr 18, 2026
@MandeepT19
Copy link
Copy Markdown
Contributor Author

Prefer unit tests over Android tests

I have looked into converting this for unit test using Robolectric. But floatingActionMenu is private in DeckPicker so I can't directly call hideFloatingActionButton() or showFloatingActionButton() to set up test state. Could you advise on the best approach should I make these methods accessible for testing, or is there a different entry point you have in mind?

@david-allison
Copy link
Copy Markdown
Member

@VisibleForTesting is typically fine

@MandeepT19
Copy link
Copy Markdown
Contributor Author

@VisibleForTesting is typically fine

Hi,
I've added @VisibleForTesting to floatingActionMenu and am testing through RobolectricTest. However floatingActionMenu is not initialized when the test runs onCreate appears to exit early before reaching initialization. I have tried bypassing the intro screen check via sharedPrefs but the issue persists. Could you advise on the correct setup for testing DeckPicker with Robolectric?

@david-allison
Copy link
Copy Markdown
Member

Sorry, I won't have the capacity to dive into this one.

@MandeepT19
Copy link
Copy Markdown
Contributor Author

Sorry, I won't have the capacity to dive into this one.

No worries at all,
I really appreciate you taking the time to look at it. I've continued investigating and the specific blocker is that floatingActionMenu remains uninitialized despite advancing the Robolectric looper and bypassing the intro screen check via sharedPrefs. The Activity lifecycle completes through .create().start().resume().visible() but the property never gets set. Will continue investigating any pointers on DeckPicker specific Robolectric setup would be appreciated when you have capacity.

@MandeepT19
Copy link
Copy Markdown
Contributor Author

And I have done further investigation: I explored testing DeckPickerFloatingActionMenu in isolation but showFloatingActionButton is private and constructor requires ActivityHomescreenBinding and DeckPicker making isolation difficult too. Would making showFloatingActionButton internal with @VisibleForTesting be acceptable?

@DoomsCoder
Copy link
Copy Markdown
Contributor

Hi @MandeepT19 , regarding the @NeedsTest annotation, this part of the cleanup tracked in issue #13283. As @david-allison noted in the main issue, we should prioritise unit test over Android instrumented tests to keep the build fast.

Could you take quick look and maybe link that issue in your PR description?

@MandeepT19
Copy link
Copy Markdown
Contributor Author

Thanks @DoomsCoder,
For linking the issue have added #13283 to the PR description.

@MandeepT19
Copy link
Copy Markdown
Contributor Author

MandeepT19 commented Apr 21, 2026

Hi @david-allison,
Tests have been reimplemented as Robolectric unit tests in /test as requested.

In the previous blocker it was @Config(application = EmptyApplication::class)and this prevented AnkiDroidApp.isInitialized from being true, causing showedActivityFailedScreen to exit onCreate early before floatingActionMenu was initialized. Removing EmptyApplication from @Config resolves this.

Two tests now pass on JVM without an emulator:
showFloatingActionButtonMakesFabVisible
hideFloatingActionButtonMakesFabGone

I have also marked floatingActionMenu as internal with@VisibleForTesting in DeckPicker.kt as suggested

@MandeepT19 MandeepT19 force-pushed the test/deck-picker-floating-action-menu branch 4 times, most recently from d23be82 to e3766e0 Compare April 21, 2026 20:00
@MandeepT19 MandeepT19 marked this pull request as draft April 21, 2026 20:13
@MandeepT19 MandeepT19 force-pushed the test/deck-picker-floating-action-menu branch from e3766e0 to e0ac735 Compare April 21, 2026 20:16
@MandeepT19 MandeepT19 marked this pull request as ready for review April 21, 2026 20:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

@github-actions github-actions Bot added the Stale label May 5, 2026
@MandeepT19
Copy link
Copy Markdown
Contributor Author

Hi @david-allison,

Just a reminder for getting this reviewed, if you you are free for it as its been nearly 2 weeks since the code is updated with instructions

@github-actions github-actions Bot removed the Stale label May 5, 2026
@david-allison david-allison removed Needs Author Reply Waiting for a reply from the original author Has Conflicts labels May 5, 2026
@david-allison
Copy link
Copy Markdown
Member

david-allison commented May 5, 2026

Sorry for the delay (in future, please ping, I have far too many reviews, but it's unusual to be waiting more than a couple of days), but I don't understand. This doesn't seem to test either of the methods:

@NeedsTest("reimplement: doubleTapAddsNote; singleTapTogglesFab")

The implementation of these tests are available: a3ce935

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