Skip to content
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

change collectJob to backgroundScope coroutine #1585

Merged
merged 4 commits into from
Sep 23, 2024

Conversation

saeedishayan76
Copy link
Contributor

Hello,
‌Based on the latest document available in developer.android, we can use backgroundScope to collect flows instead of making a job object and canceling it after the final assertion.
With that, we do not need to manually cancel the created job, and the cancellation of this coroutine is done automatically before the end of the test. The following links are about the example on the site of developer.android and kotllinlang:

https://developer.android.com/kotlin/flow/test
Screenshot 2024-09-02 at 8 04 01 PM

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-test/kotlinx.coroutines.test/-test-scope/background-scope.html

@Jaehwa-Noh
Copy link
Contributor

I'd also tried that approach before, but I've got fail on unit tests.
Did you pass the unit tests on this PR?

@saeedishayan76
Copy link
Contributor Author

I'd also tried that approach before, but I've got fail on unit tests. Did you pass the unit tests on this PR?

yes , i check unit test (in feature) after change in android studio and all test passed.

@alexvanyo alexvanyo self-requested a review September 13, 2024 23:55
@saeedishayan76
Copy link
Contributor Author

Hi @alexvanyo,
Thank you for reviewing this PR last week. I wanted to follow up and see if there are any updates or additional actions needed from my side. Are there any changes or clarifications you'd like me to make based on your review?
I'm eager to move this forward and am happy to address any concerns or make any necessary adjustments. If everything looks good, would it be possible to get your approval for merging?
I appreciate your time and attention to this PR.
Thank you!

Copy link
Contributor

@alexvanyo alexvanyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making these changes! I think this definitely helps improve the readability of these tests.

@alexvanyo alexvanyo merged commit f42afb5 into android:main Sep 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants