Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard

[Cleanup]: Add Tests to the code (`@NeedsTest`)

Open david-allison opened this issue 2 years ago • 14 comments

[!NOTE] This issue does not need to be assigned to you before you work on it

A number of paths of the code have been identified as requiring automated testing.

Typically this is regression testing: if a bug has been found, then it's useful to have a unit test which would have failed due to the bug.

  1. Find a usage of @NeedsTest: https://github.com/ankidroid/Anki-Android/blob/main/AnkiDroid/src/main/java/com/ichi2/annotations/NeedsTest.kt
  2. Post the test(s) that you're handling on this issue, so people don't duplicate work
  3. Write a unit test (/test, not /androidTest)
  4. Ensure that re-introducing the bug or issue would break the test
  5. Submit a Pull Request

david-allison avatar Feb 16 '23 21:02 david-allison

please assign this issue to me

Prithvi101 avatar Feb 17 '23 08:02 Prithvi101

Multiple people can work on this issue, please pick a test and let us know!

david-allison avatar Feb 17 '23 15:02 david-allison

I suggest clarifying @NeedsTest comments. It is often hard to understand what exactly needs to be tested, and why. For example,

  • @NeedsTest("New User: Accepts permission") in StartupStoragePermissionManager. It is not clear what needs to be tested. A test tests behavior, that's when we do x and expect y. Here the comment doesn't specify y, and does a poor job of describing x. Where do we start? StartupStoragePermissionManager doesn't have a definitive entry point. Where do we finish? From reading the code I'm assuming that we expect onRegularStartup() to be called, but I am not sure.

  • @NeedsTest("Ensure a toast is shown on a successful action") in CreateDeckDialog. Here buth x and y are specified well enough, but I don't see the reason we need this test. We are not asking to test that creating a deck can succeed, but only that a toast is shown. Assuming there's code like if (success) showToast(), such a test would be testing Kotlin's if statement, which is, I'm assuming, was not what the annotation author wanted to test.

  • In a similar fashion, @NeedsTest("Selecting APKG allows multiple files") in ImportFileSelectionFragment would be testing Android APIs, not the app.

  • @NeedsTest("removing JvmOverloads should fail"). Removing JvmOverloads is not behavior of the app or user, but of the programmer. What would the test be like? Can we make a test that would somehow recompile the app, removing JvmOverloads, and then run it in an emulator and verify that it crashes? Suppose this test fails, how does this help us?

oakkitten avatar Feb 17 '23 20:02 oakkitten

I am intrested to look into this issue ,Please assign this issue to me

Shubham-Patel07 avatar Mar 13 '23 12:03 Shubham-Patel07

This issue does not need to be assigned to you before you work on it

david-allison avatar Mar 13 '23 18:03 david-allison

Question: are unit tests just preferred over UI tests or is it a hard requirement?

It seems to my Android-newbie eye that a lot of @NeedsTest annotations request a test that validates some UI flow, testable with Espresso. Examples: https://github.com/ankidroid/Anki-Android/blob/main/AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt#L188-L194

Or am I misunderstanding something?

tomaszgarbus avatar May 05 '23 21:05 tomaszgarbus

Hello 👋, this issue has been opened for more than 3 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar Aug 03 '23 21:08 github-actions[bot]

Hey :), I've written a test for:

protected open fun editCard(fromGesture: Gesture? = null) {

to cover that inverse animations are being correctly assigned from gestures.

https://github.com/ankidroid/Anki-Android/pull/14337

roryjones123 avatar Aug 28 '23 12:08 roryjones123

I'm working on @NeedsTest("constant values are unique").

WPum avatar Mar 22 '24 04:03 WPum

I'm taking up this https://github.com/ankidroid/Anki-Android/blob/7a4dfd093db8cc794b6f8b5f63e9f0a846b58985/AnkiDroid/src/main/java/com/ichi2/anki/IntroductionActivity.kt#L44

neeldoshii avatar Mar 31 '24 17:03 neeldoshii

I'll work on this https://github.com/ankidroid/Anki-Android/blob/80487ca9771d309eeee7ad825e37e9582b2a57d7/AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptions.kt#L41

cshcsh3 avatar Oct 20 '24 06:10 cshcsh3

Hi, I'm handling the following test:

https://github.com/ankidroid/Anki-Android/blob/0d8a5c44df610a39dfc249db358216a67b5ac0e5/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt#L238

danilodanicomendes avatar Aug 10 '25 11:08 danilodanicomendes

I work on @NeedsTest("searchView == null -> return early & ensure no snackbar when the screen is opened")

nikitAugsburg avatar Oct 09 '25 16:10 nikitAugsburg

i will work on @NeedsTest("Test functionality which calls this") in file cards.kt

aladdin-afk avatar Dec 10 '25 13:12 aladdin-afk

I'm working on this

https://github.com/ankidroid/Anki-Android/blob/60fae6e6f2d1071480d103749fa7715b810ecf46/AnkiDroid/src/main/java/com/ichi2/preferences/NumberRangePreferenceCompat.kt#L189

pankaj0695 avatar Dec 27 '25 16:12 pankaj0695