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

Fix part of #89: Introduce test dispatchers support in espresso #1623

Merged
merged 13 commits into from
Aug 12, 2020

Conversation

BenHenning
Copy link
Member

Fixes part of #89 by introducing proper support for test coroutine dispatchers in Espresso.

This piggybacks off of the solution introduced in #1276 for Robolectric. That PR allows Robolectric tests in app module to share dependencies with production components by creating a test application & telling Robolectric to use it instead of OppiaApplication via an @Config annotation.

This PR achieves the same thing by using a custom test runner that reads the same annotation and forces Espresso & instrumentation to use the test application instead of the default OppiaApplication. This setup will be easier once #59 is finished since we can specify the application in per-test manifests that both Robolectric & Espresso will respect.

Note that while this lets the same test coroutine dispatchers work in both production & test code for Espresso, some additional work was needed to ensure the coroutines behave correctly. In particular, the coroutines use a fake system clock in Robolectric that requires explicit synchronization points in the tests to allow the clock to move forward & properly coordinate execution between background & main thread operations. However, in Espresso, since everything is using a real clock an idling resource is the preferred way to synchronize execution: it allows the background coroutines to operate in real-time much like they would in production, and then notify Espresso when completed. The test dispatchers API is now setup to support synchronization mechanisms for both Robolectric & Espresso (the idling resource does nothing on Robolectric and the force synchronization effectively does nothing on Espresso).

The first test being demonstrated as now stable on both Robolectric & Espresso is SplashActivityTest (as part of downstream work in #1397).

This piggybacks off of the solution introduced in #1276 for Robolectric.
That PR allows Robolectric tests in app module to share dependencies
with production components by creating a test application & telling
Robolectric to use it instead of OppiaApplication via a @config
annotation.

This PR achieves the same thing by using a custom test runner that reads
the same annotation and forces Espresso & instrumentation to use the
test application instead of the default OppiaApplication. This setup
will be easier once #59 is finished since we can specify the application
in per-test manifests that both Robolectric & Espresso will respect.

Note that while this lets the same test coroutine dispatchers work in
both production & test code for Espresso, some additional work was
needed to ensure the coroutines behave correctly. In particular, the
coroutines use a fake system clock in Robolectric that requires explicit
synchronization points in the tests to allow the clock to move forward
& properly coordinate execution between background & main thread
operations. However, in Espresso, since everything is using a real clock
an idling resource is the preferred way to synchronize execution: it
allows the background coroutines to operate in real-time much like they
would in production, and then notify Espresso when completed. The test
dispatchers API is now setup to support both synchronization mechanisms
for both Robolectric & Espresso (the idling resource does nothing on
Robolectric and the force synchronization effectively does nothing on
Espresso).

The first test being demonstrated as now stable is SplashActivityTest
(as part of downstream work in #1397.
@BenHenning
Copy link
Member Author

/cc @MohamedMedhat1998 since you ran into a situation where coordination synchronization in an Espresso test would be helpful.

Copy link
Contributor

@vinitamurthi vinitamurthi left a comment

Choose a reason for hiding this comment

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

Overall LGTM, I had a few questions on how the tests are going to use this

@vinitamurthi vinitamurthi removed their assignment Aug 12, 2020
@BenHenning
Copy link
Member Author

Thanks @vinitamurthi! Do you want to resolve your comment threads or is it fine to check in this PR as-is?

@vinitamurthi
Copy link
Contributor

Resolved my comments, thanks!

@vinitamurthi vinitamurthi removed their assignment Aug 12, 2020
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM. Really great implementation.

@rt4914 rt4914 assigned BenHenning and unassigned rt4914 Aug 12, 2020
@BenHenning
Copy link
Member Author

Thanks everyone! Going to go ahead and merge this to unblock #1624.

@BenHenning BenHenning merged commit f79d7cc into develop Aug 12, 2020
@BenHenning BenHenning deleted the introduce-test-dispatchers-support-in-espresso branch August 12, 2020 08:11
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.

4 participants