-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adjust PlayLog test setup #2
Merged
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
This allows idiomatic usage when creating MockResponses for a custom dispatcher
…tion This is safer in case the constructor ever triggers network calls.
This means refactoring DI to inject a TestScope for DefaultEventReporter so we can perform assertions on what we send to EventProducer reliably. Some of the new symbols are introduced in other modules, so they have been prefixed to indicate that they are there because of the PlayLogTest. Other renames have ocurred for consistency with this. The new DI setup allows providing DefaultEventReporter with a TestScope, meaning that the for-now only test now runs inside a runTest block, and its wait for MediaProductEnded switches out the CoroutineContext away from the test one into a regular one so that the wait actually happens since we need playback to occur for real.
This is to avoid side-effects and for readability.
For a Credentials instance to be considered logged in it needs to have non-null both userId and token. Since we need our instance to be logged in, we need to respect that.
We do not depend on it being valid so making it invalid will raise errors if that accidentally changes.
This ensures we behave correctly when certain values are null.
stoyicker
force-pushed
the
jantonio/playlog_test_setup
branch
from
May 28, 2024 08:36
96efc31
to
123080e
Compare
kgrevehagen
reviewed
May 28, 2024
player/src/androidTest/kotlin/com/tidal/sdk/player/playlog/PlayLogTest.kt
Show resolved
Hide resolved
kgrevehagen
reviewed
May 28, 2024
player/src/androidTest/kotlin/com/tidal/sdk/player/playlog/PlayLogTest.kt
Show resolved
Hide resolved
kgrevehagen
reviewed
May 28, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a couple of questions!
stoyicker
force-pushed
the
jantonio/playlog_test_setup
branch
from
May 29, 2024 07:44
123080e
to
c1aff26
Compare
kgrevehagen
approved these changes
May 29, 2024
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.
See commit messages