-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fixes: the unit test failures after coroutine version update #18589
Fixes: the unit test failures after coroutine version update #18589
Conversation
* Fixes: test failures in UnifiedCommentsEditViewModelTest after coroutine update
* Fixes: test failures in DomainSuggestionsViewModelTest after coroutine update
* Fixes: test failures in BackupDownloadViewModelTest after coroutine update
* Fixes: test failures in QRCodeAuthViewModelTest after coroutine update
* Fixes: test failures in SiteCreationDomainsViewModelTest after coroutine update
* Fixes: test failures in SiteCreationProgressViewModelTest after coroutine update
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
* Fixes: test failures in PagesViewModelTest after coroutine update
* Fixes: test failures in PagesViewModelTest after coroutine update
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.
Thanks for this work and the reason it started: that you felt brave and proved to be right when tackling the Coroutines update work 🙇🏻 !
I found a quickfix for the issue we were having in DomainSuggestionsViewModelTest
and suggested a change via a patchfile 👍🏻
Everything else LGTM, but IIRC there were more failing tests at CI time, probably the PR is not fully ready. For that reason (my uncertainty) I didn't approve it just yet 👍🏻
WordPress/src/test/java/org/wordpress/android/ui/domains/DomainSuggestionsViewModelTest.kt
Outdated
Show resolved
Hide resolved
* Fixes: test failures in DomainSuggestionsViewModelTest after coroutine update
@AjeshRPai I'm with Ovi on this one. I pulled this branch and tried the first failing test (as reported by CI) |
Hey folks 👋🏻 , Fyi I've started on working to fix these tests yesterday, sharing with you in hope we don't duplicate the same efforts 🙏🏻 ! I've managed to find some small fixes that made all tests in Mostly doing this to see if the new coroutines version fixes our mutex crash bug (#18589) 😄 |
…kotlinxCoroutinesVersion-1.7.1-suggestions Fix failing tests in parent PR #18589
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.
Should be ready when CI passes 🤞🏻
Generated by 🚫 dangerJS |
Fixes
Fixes the failure of the unit tests after the coroutines library update. PR - #18500
On Kotlin coroutines library version 1.6.4
If the coroutine inside a function throws an exception, the test passes, and the exception is thrown silently to the console. In some cases, if the test is asserting a return value and there is a side effect function that throws an exception, then the test still passes.
On Kotlin coroutines library version 1.7.1
The exceptions inside the production code will be thrown, regardless of whether the test is passing, making the unit tests fail.
More details in this thread - Kotlin/kotlinx.coroutines#1205
To test:
Regression Notes
Potential unintended areas of impact
Incorrect unit test logic
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: