Add unit tests for NumberRangePreferenceCompat dialog behavior (#13283) #19957

pankaj0695 · 2025-12-28T13:14:24Z

Purpose / Description

This PR adds missing unit test coverage for NumberRangePreferenceCompat, specifically the behavior marked with @NeedsTest where the preference value should remain unchanged when the dialog is confirmed with blank input. It also adds coverage for range validation helpers to prevent regressions.

Fixes

Fixes [Cleanup]: Add Tests to the code (@NeedsTest) #13283

Approach

Added a new Robolectric test class at AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt.
Implemented focused unit tests for:
- getValidatedRangeFromInt: values within range and below min clamping.
- getValidatedRangeFromString: empty string, valid numeric string and invalid string handling.
- onDialogClosed: ensures blank input and cancelled dialog do not trigger value updates, while valid input does.

How Has This Been Tested?

Run the unit tests via Gradle

./gradlew :AnkiDroid:testDebugUnitTest --tests "com.ichi2.preferences.NumberRangePreferenceCompatTest"

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

welcome · 2025-12-28T13:14:27Z

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.

david-allison

Looks good, thanks! Tests are great!

Feel free to resolve the nitpicks without changes, especially the mockk one, as that may not easily be feasible.

In addition: could you squash any new comments and change the commit message to something along the lines of:

test: add NumberRangePreferenceCompatTest

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

david-allison · 2025-12-28T15:49:27Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+class NumberRangePreferenceCompatTest {
+    private lateinit var context: Context
+    private lateinit var preference: TestableNumberRangePreferenceCompat
+
+    @Before
+    fun setUp() {
+        context = ApplicationProvider.getApplicationContext()
+        preference = TestableNumberRangePreferenceCompat(context)
+    }


class NumberRangePreferenceCompatTest: RobolectricTest() { private val preference = TestableNumberRangePreferenceCompat(targetContext)

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

david-allison · 2025-12-28T15:51:15Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+    /**
+     * Test subclass to expose onDialogClosed for testing and track setValue calls
+     */
+    private class TestNumberRangeDialogFragmentCompat : NumberRangePreferenceCompat.NumberRangeDialogFragmentCompat() {


nit: You may be able to use mockk here instead: https://mockk.io/

pankaj0695 · 2025-12-28T19:11:08Z

Thanks for the review, I addressed the nitpicks by extracting the repeated dialog setup into a helper and switching the onDialogClosed assertions to MockK interaction verification (no functional changes). I also squashed the commits and updated the commit message.

criticalAY · 2025-12-29T07:54:50Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+    // Keep the real preference for validation tests
+    private val preference = TestableNumberRangePreferenceCompat(targetContext)
+
+    // Use a mock for onDialogClosed interaction verification
+    private val preferenceMock: NumberRangePreferenceCompat = mockk(relaxed = true)
+
+    // Tests for getValidatedRangeFromInt


LLM? but I am not opposing using it just that the code before being pushed should be cleaned and understood, here you have left comments that are not needed

criticalAY · 2025-12-29T07:59:14Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+    // Test for onDialogClosed behavior
+    // This tests: @NeedsTest("value is set to preference previous value if text is blank")


criticalAY · 2025-12-29T08:01:40Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+
+        // Inject the preference into the fragment (preference is held by the base class)
+        // We use reflection so we can unit-test onDialogClosed without running a full Preference/Fragment lifecycle.
+        val preferenceField = androidx.preference.PreferenceDialogFragmentCompat::class.java.getDeclaredField("mPreference")


criticalAY · 2025-12-29T08:02:32Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+    }
+
+    // Test for onDialogClosed behavior
+    // This tests: @NeedsTest("value is set to preference previous value if text is blank")


If you are resolving an annotation then remove that annotation from the original place so that we know what is being resolved

pankaj0695 · 2025-12-29T10:04:31Z

Thanks for the note, I did use LLM for guidance, but I made sure I understood the changes before pushing. I’ve now removed the unnecessary comments and made required changes, and will make sure future updates are clean and reviewed before I push them.

criticalAY

Looks good one last change

criticalAY · 2025-12-29T10:19:46Z

AnkiDroid/src/test/java/com/ichi2/preferences/NumberRangePreferenceCompatTest.kt

+    private val preferenceMock: NumberRangePreferenceCompat = mockk(relaxed = true)
+
+    @Test
+    fun getValidatedRangeFromInt_withinRange_returnsInput() {


Use backticks or camel case

criticalAY · 2025-12-29T10:22:42Z

@pankaj0695 Please clean the commit message

made changes make required changes update

pankaj0695 · 2025-12-29T10:38:33Z

I have made the required updates

david-allison requested changes Dec 28, 2025

View reviewed changes

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

pankaj0695 force-pushed the created_NumberRangePreferenceCompatTest.kt branch from 8e2c1b7 to 581fdfe Compare December 28, 2025 19:05

sanjaysargam added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Dec 29, 2025

criticalAY requested changes Dec 29, 2025

View reviewed changes

criticalAY added the Needs Author Reply Waiting for a reply from the original author label Dec 29, 2025

pankaj0695 force-pushed the created_NumberRangePreferenceCompatTest.kt branch from 581fdfe to b8eac7b Compare December 29, 2025 10:01

criticalAY requested changes Dec 29, 2025

View reviewed changes

add NumberRangePreferenceCompatTest

8300a01

made changes make required changes update

pankaj0695 force-pushed the created_NumberRangePreferenceCompatTest.kt branch from b8eac7b to 8300a01 Compare December 29, 2025 10:37

pankaj0695 requested review from criticalAY and david-allison December 30, 2025 09:28

		// Test for onDialogClosed behavior
		// This tests: @NeedsTest("value is set to preference previous value if text is blank")

Uh oh!

Add unit tests for NumberRangePreferenceCompat dialog behavior (#13283) #19957

Are you sure you want to change the base?

Add unit tests for NumberRangePreferenceCompat dialog behavior (#13283) #19957

Conversation

pankaj0695 commented Dec 28, 2025

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Uh oh!

welcome bot commented Dec 28, 2025

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david-allison Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david-allison Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

pankaj0695 commented Dec 28, 2025

Uh oh!

criticalAY Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

criticalAY Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

criticalAY Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

criticalAY Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

pankaj0695 commented Dec 29, 2025

Uh oh!

criticalAY left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalAY Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

criticalAY commented Dec 29, 2025

Uh oh!

pankaj0695 commented Dec 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

criticalAY left a comment •

edited

Loading