Ressurect instrumented tests #8
Merged
Conversation
This file contains 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
Current implementation of offline scrobbling is to save all required data to submit scrobble in local shared preferences file and when back online, DSub sends them to server. When in Offline mode, dsub uses pathes to local files as their identifies as opposite to Online mode where server side identifies are utilized. In order to send scrobble it is required to specify server side ID in a query. Thus, Dsub has to match identifies of what it has scrobbled while offline (paths to files in file system) to server side identifies. To do so, it relies on 1. Querying local database, which contains mapping local path to file <-> server side id. It sounds good but we don't live in perfect world and there are couple flaws in this implementation: a) Dsub's local database only contains information for songs that have been played in online mode at least once. b) It just doesn't work since in scrobble OfflineMusicService.scrobble method id of a song comes with ".complete" postfix and in database it's saved without this ".complete" postfix so it fails to lookup server side id. 2. Querying the server with lucene syntax search clause. Subsonic api doesn't say anything about lucene search syntax support and not all servers have support for it (i.e. see this issue for navidrome navidrome/navidrome#1466). Besides, it isn't precise in case you have multiple songs with same artist and title (i.e. multiple editions of an album). This commit and according PR focus on fixing problems with looking up local path to file <-> server side id. It is achieved with removing ".complete" substring from all identifies which comes to OfflineMusicSerivce.scrobble. And by adding all cached songs to local db on download stage so when DSub switches to offline mode it always has server side identifies for all media files it has locally.
All tests pass except testGetRecentDownloadsWithoutPlaylist
|
sorry for the delayed merging - I somehow ended up not "watching" my own repo and got no notifications.. |
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.
Closes #7.
Basically, it wanted to have kotlin standard library.
Also, there is a helper method createMusicSongs which is used to provide songs for tests.
This method used to return hand-crafted song entry which actually exists on subsonic test server.
But now navidrome test server is utilized in the project by default, so some tests was getting this song,
failed to lookup it up on navidrome demo server and thus, some tests fails.
I redone this method to pull a really present on server song, it would allow us to run tests against different subsonic servers in future.
All tests pass except testGetRecentDownloadsWithoutPlaylist. I still don't know why, have looked up changes in related to this test code, tried to rollback them and still couldn't figure out what's wrong.
The problem actually not the test's assertion, but exception is being throw inside application code:
P.S. All my pull requests are based one on another, I'm doing it because it simplifies for me building app with all my changes for personal use. I guess there would be no problems if you merge my PRs in submit order. If this is gonna cause problems, let me now and we'll figure something out.