Improve HistoryRecordManager tests #7700
Conversation
app/src/androidTest/java/org/schabi/newpipe/local/history/HistoryRecordManagerTest.kt
Outdated
Show resolved
Hide resolved
XiangRongLin
added
the
codequality
| // shuffle to make sure the order of items returned by queries depends only on | ||
| // SearchHistoryEntry.creationDate, not on the actual insertion time | ||
| database.searchHistoryDAO().insertAll(relatedSearches.shuffled()) |
There was a problem hiding this comment.
This is a very subjective view, so feel free to ignore it:
I finally figured out, why this stood out to me. It ensures that the ORDER_BY_CREATION_DATE inside the dao is working but does it implicitly in every test instead of getting its own dedicated test. The 2 lines of comments are needed to convey this implicit test. In my personal opinion this should be put into its own test and then the helper method can also be removed as I see 2 lines as an acceptaple duplication
There was a problem hiding this comment.
Nice catch. I think order should be enforced in each test, since each test checks that the correct items are returned and in the correct order in many situations. Therefore I'll leave the code as it is and add a comment explaining that shuffling the items is used to verify the ORDER BY works. I pushed, please approve again ;-)
|
LGTM |
|
Kudos, SonarCloud Quality Gate passed!
|
What is it?
Description of the changes in your PR
This PR improves the
HistoryRecordManagergetRelatedSearchestests by shuffling items before inserting them and adding a newgetRelatedSearches_emptyQuery_manyDuplicatestest.Fixes the following issue(s)
Nothing, but check out #7553 and #7491 for context
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence