Skip to content

test: unit test for CardTemplateEditor#19877

Merged
lukstbit merged 1 commit intoankidroid:mainfrom
criticalAY:test/save-button
Feb 7, 2026
Merged

test: unit test for CardTemplateEditor#19877
lukstbit merged 1 commit intoankidroid:mainfrom
criticalAY:test/save-button

Conversation

@criticalAY
Copy link
Contributor

Purpose / Description

When a user attempted to save changes in the CardTemplateEditor, the save button was typically disabled to prevent double-submissions. However, if an unexpected exception occurs during the save logic, the button remained disabled, forcing the user to restart the activity. This change adds a regression test to ensure the button is always restored to an enabled state regardless of success or failure. resolves a TODO from my previous PR

Fixes

NA

Approach

NA

How Has This Been Tested?

Local test

Learning (optional, can help others)

NA

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
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.

I don't like requesting changes on test code, hopefully less frustrating as this is a patch 😅


shadowOf(Looper.getMainLooper()).idle()

assertTrue("Button should be clickable after failure", confirmButton.isClickable)
Copy link
Member

Choose a reason for hiding this comment

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

If feasible, we should confirm that an error occurs, this informs us if someone 'breaks' the test by 'breaking' the failure simulation of:

        // throw an exception to simulate failure
        testEditor.tempNoteType = null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried multiple ways the error is there but since we are throwing it the test fails due the reason I stated I would leave it for now, we can create more tests later for now something is better than nothing

Copy link
Member

@david-allison david-allison Dec 21, 2025

Choose a reason for hiding this comment

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

Sure, I didn't expect much effort to be put here, if it's not easy we can come back to it later

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 20, 2025
@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author labels Dec 22, 2025
- Ensure save button re-enables on exception

Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com>
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

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

I fixed the conflict to move things forward.

I kept the original code, we use binding now but as this uses a menu item I didn't want to complicate things(we are also currently modifying this screen so the tests might be changed as well).

@lukstbit lukstbit added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge Has Conflicts labels Feb 7, 2026
@lukstbit lukstbit enabled auto-merge February 7, 2026 03:44
@lukstbit lukstbit added this pull request to the merge queue Feb 7, 2026
Merged via the queue into ankidroid:main with commit 26b0abb Feb 7, 2026
15 checks passed
@github-actions github-actions bot added this to the 2.24 release milestone Feb 7, 2026
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Feb 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants