Skip to content

Test getFileCachedCopy returns absolute path#20441

Merged
david-allison merged 1 commit intoankidroid:mainfrom
manocormen:test-getfilecachedcopy
Mar 11, 2026
Merged

Test getFileCachedCopy returns absolute path#20441
david-allison merged 1 commit intoankidroid:mainfrom
manocormen:test-getfilecachedcopy

Conversation

@manocormen
Copy link
Contributor

Purpose / Description

Regression test that ensures that getFileCachedCopy in ImportUtils.kt returns a regular absolute path, as per the @NeedsTest.

Fixes

Relates to #13283.

Approach

I looked at the older getFileCachedCopy. It used to return a URI-encoded path. It was updated to return a regular absolute filepath. That's when @NeedsTest was added.

To discriminate between old/new behavior, I used a filename with a space, since they have a particular URI-encoding. If getFileCachedCopy were to return a URI-encoded path again, the test would catch the regression.

I used the already-implemented TestFileImporter to handle method dependencies and isolate the behavior under test.

How Has This Been Tested?

./gradlew jacocoUnitTestReport
./gradlew lintRelease
./gradlew ktlintCheck
./gradlew assembleDebug

Learning

A couple of things I did that helped:

  • Contrasting the old/new method helped clarify the motivation behind the @NeedsTest.
  • Checking for helpers (TestFileImporter) helped save time and keep the commit short.

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 NA
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens NA
  • UI Changes: You have tested your change using the Google Accessibility Scanner NA

In older code, `getFileCachedCopy` returned a URI-encoded filepath.
Later, it was updated to return a regular absolute filepath. This
test enforces that behavior. I used a filename with a space because
these have a particular URI-encoding (`%20`), so it should catch
any regression.
@welcome
Copy link

welcome bot commented Mar 11, 2026

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.

@manocormen manocormen marked this pull request as ready for review March 11, 2026 15:31
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.

Looks great!

@david-allison david-allison added this pull request to the merge queue Mar 11, 2026
Merged via the queue into ankidroid:main with commit 4c369ea Mar 11, 2026
19 checks passed
@github-actions github-actions bot added this to the 2.24 release milestone Mar 11, 2026
@manocormen manocormen deleted the test-getfilecachedcopy branch March 11, 2026 18:57
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.

2 participants