Update unit tests for one-isolate branch

[ryanheise]

Is your feature request related to a problem? Please describe.

The unit tests are not in working order on the one-isolate branch.

Describe the solution you'd like

Migrate the unit tests to work with the new audio_service API.

Describe alternatives you've considered

None

Additional context

None

[suragch]

This is something I might be able to help with. If you would like help implementing the tests, are there any special instructions?

Note to self: link to tests

[ryanheise]

Sure! You are welcome to take a stab at it while I take on another task. Now that we have a platform interface, you ought to be able to mock that with a mock implementation of that interface or mockito instead of trying to mock the method channel. Let me know if you have any questions.

[nt4f04uNd]

(update): today-tomorrow will end the _IsolateAudioHandler and add a bunch of integration tests for it with flutter_isolate, or unit, if that's feasible, will see

[suragch]

Ok, so I'm looking at this today and I guess I don't really know how to start. I would need a walk-through but by that time one of you might be able to finish. I'm still willing to try, but I'd probably at least need a more detailed list of steps to follow.

[nt4f04uNd]

Synchronizing subjects between isolates is much harder task than I first thought initally.. What I currenly have is that I tried on both ends to listen to both stream and port that receives messages from the other isolate, but this obviously leads to that messages are sent back and forth infinitely by this scheme:

1. subject has listener that sends message to other isolate
2. in that isolate message is added into the subject
3. because this subject is also listened the event is send back to the sender isolate
4. isolate receives event and calls add
5. back to 1

Any thoughts?

[nt4f04uNd]

Tests for the isolate are mostly done, except for subjects

[nt4f04uNd]

Ah, that's easy, I will just filter these messages out lol

[nt4f04uNd]

@suragch I will give you some steps a bit later, once I'm done with what I'm currently doing, since this will give a ground for other tests, such as MockBaseAudioHandler

[nt4f04uNd]

@suragch I pushed my changes in #735, you can check them out for example, I'm testing IsolateAudioHandler there. Other unit tests will require writing something similar, but with other handlers.

You can also checkout #726 if you want

Here are some sections we will need to cover

Platform connection

I think that we will need to expose the _HandlerCallbacks for testing a communication with AudioServicePlatform. Tests would go like this:

1. At the very beginning we create our newly exposed AudioHandlerCallbacks class and create a MockBaseAudioHandler
2. Then for each method of the callbacks we call it with specific parameters and expect the MockBaseAudioHandler to be called also with specific parameters and return a specific value, if it can

Other handlers

• CompositeAudioHandler
• BaseAudioHandler - only ensure default method implementations do what they do
• SwitchAudioHandler

Data classes

Classes like MediaItem, Rating, PlaybackState, etc. - ensure they are translated into platform messages and they generally work correctly

I think you should still wait a little bit though, until @ryanheise can review my PRs

[suragch]

@nt4f04uNd Thank you for your outline. That was really helpful.

I started some work on this. I added tests for BaseAudioHandler. Would someone mind taking a look and telling me if I'm on the right track:

• https://github.com/suragch/audio_service/blob/aab806ad1434147a7cb68ce0599a894dc217a48a/audio_service/test/audio_service_test.dart

[ryanheise]

Awesome @suragch ! Can you create a draft pull request? I can then use GitHub's facilities to do a review.

[nt4f04uNd]

@ryanheise I'm planning to test the data classes. Do you think we should add tests for enums and enum-like classes as well?

[ryanheise]

All we really need is to test each method has the expected outcome - enums themselves don't have methods, so I think their coverage would be incidental rather than the focus of specific unit tests.

[suragch]

@nt4f04uNd If you are testing the data classes, one thing to note is PlaybackState.copyWith is missing the updateTime parameter.

[nt4f04uNd]

I also have a platform connection tests written locally which are based of #735

[suragch]

I meant to keep working on this but got busy with work. What is the current status of the unit test updates? Is there something I can do?

[ryanheise]

Hi @suragch , thanks so much for your previous work on this which has now been merged.

Full coverage is not ultra urgent, as I would be happy to include the current level of testing in the next stable release and then chip away at it after that.

But if you're interested to know what's left to be done, you can run flutter test --coverage to generate the coverage data, and then use a visualisation tool on the generated coverage data to reveal which lines of code may not have been tested yet. Currently we're at 47.8% coverage, and long term I'd like to get this above 70% coverage