Test for currentdeckid by aladdin-afk · Pull Request #19738 · ankidroid/Anki-Android

aladdin-afk · 2025-12-10T16:15:50Z

Purpose / Description

The tests fulfill the request by testing the methods that use currentDeckId() to ensure the correct deck configuration is applied in all scenarios

Fixes

completes one @needstest from [Cleanup]: Add Tests to the code (@NeedsTest) #13283

Approach

Create a card
Call a function that uses currentDeckId()
Verify it got the right deck configuration either did or odid

6 functions tested

1.timelimit() for normal, filtered deck
2.shouldShowTimer()
3.autoplay()
4.replayQuestionAudioOnAnswerSide()
5.timeTaken()

How Has This Been Tested?

did use ./gradlew libanki:test ./gradlew ktlintFormat

Learning (optional, can help others)

https://youtu.be/9yre-M1XwVw?si=5UwsCUsnT8DDaIqc
can watch this playllist for basics
usually we use 2 standard forms of decks, so 2 deck ids did, oid abbreviated as deck id, original deck id
currentDeckId() returns the correct deck ID to use for configuration by prioritizing oDid when non-zero, ensuring that deck settings always come from the original deck, not temporary filtered decks.

also to find needstests annotations in Android Studio :
Click on your project root in the Project panel
Press Ctrl+Shift+F or use menu: Edit → Find → Find in Files
now type: @NeedsTest
select Scope: Project Files
cheers!

Checklist

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: N/A (unit tests only)
UI Changes: N/A (unit tests only)

welcome · 2025-12-10T16:15:53Z

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

aladdin-afk · 2025-12-10T16:38:03Z

sorry for this, will continue tomorrow

mikehardy · 2025-12-10T20:08:32Z

Please run your tests locally and push when they're working
Don't just push and push and push (and generate notifications for maintainers that watch the repo) please

aladdin-afk · 2025-12-10T20:10:18Z

sorry i will look for fixing it in local

david-allison

This PR isn't testing functionality which calls currentDeckId, it's testing the implementation of currentDeckId()

aladdin-afk · 2025-12-11T08:13:04Z

oh mybad, i just realised.sorry for the wrong commit.

david-allison · 2025-12-11T22:19:04Z

@aladdin-afk Please mark as 'Ready for review' if it's ready for review. I almost skipped over the notification as tihis was a Draft 😓

david-allison

Just to add some context:

Checking git blame (annotate) on the line which adds the @NeedsTest reveals that the following code was broken:

The aim of this annotation is to be sure that there is regression cover over this scenario.

None of the previous scenarios covered this. You've now resolved this issue

No action needed here.

The git blame would guide someone to adding tests to the main implementation in NoteEditor, which would likely have caught the following regressions from appearing:

I've added tests for the first two, and the final is on my immediate TODO list, so no action is needed here

libanki/src/test/java/com/ichi2/anki/libanki/CardTest.kt

david-allison

happy, cheers! One nitpick

libanki/src/test/java/com/ichi2/anki/libanki/CardTest.kt

david-allison

Ah, forgot about the override comment, that's a minor blocker

aladdin-afk · 2025-12-13T01:18:49Z

Ah, forgot about the override comment, that's a minor blocker

if it was the helper function, i did remove it. if not pls tell i will change it.

david-allison

Cheers! Looks great

This will be merged sooner if you squash & force push the changes into a single commit, but we'll deal with this if we have time

� This is the 1st commit message: added test files for currentdeckid() mark test as flaky Issue 19729 fix(deckpicker): Added contentDescription to sync and add card buttons deprecation(libanki): prefer `defaultsForAdding()` over `current()` Deprecates Notetypes.current() in favor of col.defaultsForAdding() Fixes: 19650 Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> chore: log 'Activity with no Application' error It's likely that there's another cause other than the backup manager Diagnostics for issue 19050 style: fix dangling top-level KDocs This breaks KtLint 1.8.0 > A dangling toplevel KDoc is not allowed (cannot be auto-corrected) * modify `libs.versions.toml` to use `ktlint = '1.8.0'` * ./gradlew ktLintFormat * revert all changes * manually fix warnings, searching for 'dangling' * manually check fixes ensuring only top-level comments are processed Issue 19614 - updating ktLint docs(github): Explain workflows Quality Checks can be linked to from other pages as an onboarding guide https://github.com/ankidroid/Anki-Android/tree/main/.github/workflows/README.md#quality-checks docs(deps): improve AndroidX lifecycle-process * link changelogs * rename to 'androidxLifecycleProcess' * make location consistent with other libraries Added in 7f7cd37 Disable "Scroll toolbar" if toolbar is not shown In the note editor's overflow menu, disable the "Scroll toolbar" option if the toolbar (for HTML formatting) is not shown. Add comprehensive tests for Card.currentDeckId() - Test normal deck scenario (oDid=0 returns did) - Test filtered deck scenario (oDid takes priority) - Test precedence when both IDs are positive - Test real-world filtered deck usage - Test normal deck default behavior - Test edge case where both IDs are same Addresses @NeedsTest annotation on currentDeckId() method Update Card.kt Update CardTest.kt chore: fix warnings in ExifUtil and UniqueArrayList * Convert var to val in ExifUtil * Fix KDoc references in UniqueArrayList Issue: 13282 Co-authored-by: David Allison <62114487+david-allison@users.noreply.github.com> Add comprehensive tests for Card.currentDeckId() - Test normal deck scenario (oDid=0 returns did) - Test filtered deck scenario (oDid takes priority) - Test precedence when both IDs are positive - Test real-world filtered deck usage - Test normal deck default behavior - Test edge case where both IDs are same Addresses @NeedsTest annotation on currentDeckId() method Update Card.kt Update CardTest.kt Update Card.kt Update Card.kt Update CardTest.kt Update CardTest.kt Update CardTest.kt Update Card.kt Update CardTest.kt Update CardTest.kt Update CardTest.kt Update tests to verify functionality calling currentDeckId() Update CardTest.kt Update CardTest.kt Update CardTest.kt Update CardTest.kt Update CardTest.kt Revert "added test files for currentdeckid()" This reverts commit 1b3060e. � This is the commit message #2: Update Card.kt � This is the commit message #3: Update CardTest.kt � This is the commit message #4: Update Card.kt � This is the commit message ankidroid#5: Update CardTest.kt � This is the commit message ankidroid#6: Revert "Update Card.kt & Cardtest.kt" This reverts commit 5bc3121. Test files in cardtest.kt & card.kt

david-allison · 2025-12-22T04:15:26Z

@BrayanDSO The commit message here was weird :/

4a891b7

david-allison requested changes Dec 10, 2025

View reviewed changes

aladdin-afk mentioned this pull request Dec 11, 2025

Test for currentdeckid aladdin-afk/Anki-Android#2

Open

5 tasks

aladdin-afk requested a review from david-allison December 11, 2025 18:23

david-allison marked this pull request as ready for review December 11, 2025 22:19

david-allison reviewed Dec 11, 2025

View reviewed changes

david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 12, 2025

aladdin-afk requested a review from david-allison December 12, 2025 23:27

david-allison approved these changes Dec 13, 2025

View reviewed changes

libanki/src/test/java/com/ichi2/anki/libanki/CardTest.kt Outdated Show resolved Hide resolved

libanki/src/test/java/com/ichi2/anki/libanki/CardTest.kt Outdated Show resolved Hide resolved

david-allison requested changes Dec 13, 2025

View reviewed changes

aladdin-afk requested a review from david-allison December 13, 2025 02:29

david-allison approved these changes Dec 13, 2025

View reviewed changes

david-allison added Needs Second Approval Has one approval, one more approval to merge squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. and removed Needs Author Reply Waiting for a reply from the original author labels Dec 13, 2025

aladdin-afk force-pushed the test_for_currentdeckid branch from ff79364 to 1b3060e Compare December 14, 2025 00:58

aladdin-afk force-pushed the test_for_currentdeckid branch from 965c76b to 4a891b7 Compare December 14, 2025 02:18

david-allison removed the squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. label Dec 14, 2025

BrayanDSO approved these changes Dec 20, 2025

View reviewed changes

BrayanDSO added this pull request to the merge queue Dec 20, 2025

BrayanDSO 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 labels Dec 20, 2025

Merged via the queue into ankidroid:main with commit 259df96 Dec 20, 2025
15 checks passed

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 Dec 20, 2025

github-actions bot added this to the 2.24 release milestone Dec 20, 2025

david-allison mentioned this pull request Mar 3, 2026

Fix: Remove unused variables in CardTest.kt and DeckNodeTest.kt. #20380

Closed

5 tasks

Uh oh!

Conversation

aladdin-afk commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

Uh oh!

welcome bot commented Dec 10, 2025

Uh oh!

aladdin-afk commented Dec 10, 2025

Uh oh!

mikehardy commented Dec 10, 2025

Uh oh!

aladdin-afk commented Dec 10, 2025

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

aladdin-afk commented Dec 11, 2025

Uh oh!

david-allison commented Dec 11, 2025

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

aladdin-afk commented Dec 13, 2025

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david-allison commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aladdin-afk commented Dec 10, 2025 •

edited

Loading

david-allison commented Dec 22, 2025 •

edited

Loading