Gallery multi select test and fix #16267
Open
+231
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
PhilLab
commented
Jan 11, 2026
PhilLab
commented
- Tests written, or not not needed
The method actually returned the number of rows, but each row contains multiple files (currently: 2 or 5, depending on the display rotation). In the unit test, the expected file count was changed from the original and correct value 4 to 2 in the commit 66d8756, but the correct change is in the implementation. Hence, reinstating the old expected value and the test does pass. Signed-off-by: Philipp Hasper <vcs@hasper.info>
The crash came from a collision of the improperly coded hash function GalleryRow#calculateHashCode(). Simply summing the row's file hashes can easily cause a collision as the same sum can also be achieved by two other file hashes. Take two rows with two files each. All having the same parentId=0. Row 1: 263512 and 148830 Row 2: 279897 and 132445 Summing the hashes given by calculateHashCode() will return in the same value for Row1 and Row2. PR #15918 fixed this by migrating away from this faulty hash function. This commit tests the bugfix first with a known collision and then some random fileIds for some additional explorative testing. This commit also removes the problematic calculateHashCode(), as it is now unused. Signed-off-by: Philipp Hasper <vcs@hasper.info>
This was originally meant to find the cause of crashes caused by non-unique IDs in the ViewHolder (#15918 and #16194). But the UI test still succeeded because it didn't carefully craft the fileIds to cause the issue. Once the actual reason, a collision of an improper hash function, was found, a unit test for that scenario was added instead - see prior commit. Committing this UI test anyways, as it might be able to catch other, future regressions. Signed-off-by: Philipp Hasper <vcs@hasper.info>
PhilLab
changed the title
4 tasks
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/16267.apk |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.