Espresso idling resource for Apollo GraphQL client #469

VisheshVadhera · 2017-04-28T20:06:02Z

Closes #429

Added a new ApolloTracker class which keeps track of the running ApolloCall & ApolloPrefetch objects. ApolloIdlingResource basically uses this class (indirectly) to keep a count of the running call objects. When the count becomes zero it changes its state from active to idle.

ApolloTracker is very similar to the Dispatcher class in okHttpClient, but without the ExecutorService encapsulated inside.

… feature-espresso-idling-resource Conflicts: apollo-integration/build.gradle apollo-integration/src/test/java/com/apollographql/apollo/IntegrationTest.java apollo-runtime/src/main/java/com/apollographql/apollo/ApolloClient.java apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java

Not exposing apolloTracker to the clients. Instead they have to call apolloClient's getRunningCount method and setIdleCallback method which then delegates these calls to the apolloTracker

… feature-espresso-idling-resource

VisheshVadhera · 2017-04-29T10:22:20Z

Please put the review on hold. I just found a flaky test. Will fix it and we can then review this one.

sav007 · 2017-05-02T03:12:27Z

com.apollographql.apollo.ApolloPrefetchCallbackTest > onNetworkError[test(AVD) - 4.0.4] FAILED 
Test failed to run to completion. Reason: 'Instrumentation run failed due to 'java.lang.AssertionError''. Check device logcat for details
Tests on test(AVD) - 4.0.4 failed: Instrumentation run failed due to 'java.lang.AssertionError'
:apollo-android-support:connectedDebugAndroidTest FAILED

… feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java

…Vadhera/apollo-android into feature-espresso-idling-resource

VisheshVadhera · 2017-05-03T06:04:56Z

@sav007 Fixing it.

…Vadhera/apollo-android into feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/internal/ApolloCallTracker.java

… feature-espresso-idling-resource Conflicts: gradle/dependencies.gradle

sav007 · 2017-05-13T16:13:05Z

apollo-idling-resource/gradle.properties

@@ -0,0 +1,4 @@
+POM_ARTIFACT_ID=apollo-idling-resource


should we call it then apollo-espresso-support

Hmm, yeah adding espresso makes it clear that this module is about supporting espresso testing. apollo-idling-resource sounds vague.

sav007 · 2017-05-13T16:13:43Z

...lo-idling-resource/src/main/java/com/apollographql/apollo/espresso/ApolloIdlingResource.java

@@ -0,0 +1,61 @@
+package com.apollographql.apollo.espresso;


com.apollographql.apollo.test.espresso ?

sav007 · 2017-05-13T16:14:09Z

apollo-idling-resource/gradle.properties

@@ -0,0 +1,4 @@
+POM_ARTIFACT_ID=apollo-idling-resource
+POM_NAME=Apollo GraphQL Idling Resource


Apollo GraphQL Espresso Support

sav007

LGTM.
Only question about the naming of this module.

And I feel we need to publish jars instead of aars for all our android libs: http://stackoverflow.com/questions/19307341/android-library-gradle-release-jar

… feature-espresso-idling-resource

VisheshVadhera · 2017-05-13T20:45:40Z

@sav007 I think it makes sense to publish jars only as we don't really have any resources in our android modules. Should I open a new issue, so that this can be tackled in another PR?

VisheshVadhera · 2017-05-13T20:52:17Z

Ok, looks like the build failed because the CI server was unable to download guava :|

A problem occurred configuring project ':apollo-android-support'.

Could not resolve all files for configuration ':apollo-android-support:classpath'.
Could not download guava.jar (com.google.guava:guava:18.0)
> Could not get resource 'https://jcenter.bintray.com/com/google/guava/guava/18.0/guava-18.0.jar'.
> Response 304: Not Modified has no content!

sav007 · 2017-05-16T03:45:10Z

@digitalbuddha @BenSchwab @marwanad any comments?

marwanad · 2017-05-16T21:28:03Z

I'll take a look tomorrow.

edit: srry got swamped with other stuff. I'll get back to it in a day or two.

… feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/ApolloClient.java apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java

marwanad

👍 some minor nits.

Apologies for the delay, I'll finish up the rest of the review later.

I'm not a fan of the eager initialization of the ApolloTracker but it seems inevitable for now. Would combining both the dispatcher and tracker under one extensible class make sense for this?

Last thing, I think it's not worth creating a module for the EspressoIdlingResource under this repo though. Though unlikely, it would guard us against having to maintain that module if Espresso changes in the future.
We should, however, provide the API for an idle callback.

marwanad · 2017-05-17T19:54:13Z

apollo-espresso-support/.gitignore

@@ -0,0 +1 @@
+/build


is this needed?

The top-level .gitignore has build/ already I believe.

Not sure if the top level .gitignore ignores the module specific build directories, so yeah this is indeed required.

marwanad · 2017-05-17T19:55:26Z

...resso-support/src/main/java/com/apollographql/apollo/test/espresso/ApolloIdlingResource.java

+   * Creates a new {@link IdlingResource} from {@link ApolloClient} with a given name. Register this instance using
+   * Espresso class's registerIdlingResource in your test suite's setup method.
+   *
+   * @param name         name of this idlingResource instance.


nit:spacing

marwanad · 2017-05-23T19:40:05Z

apollo-espresso-support/gradle.properties

@@ -0,0 +1,4 @@
+POM_ARTIFACT_ID=apollo-espress-support


Nice catch!

marwanad · 2017-05-23T19:40:45Z

apollo-espresso-support/gradle.properties

@@ -0,0 +1,4 @@
+POM_ARTIFACT_ID=apollo-espress-support
+POM_NAME=Apollo GraphQL Espresso Support
+POM_DESCRIPTION=An EspressoIdlingResource for Apollo GraphQL Client.


EspressoIdlingResource -> Espresso Idling Resource?

marwanad · 2017-05-23T19:42:08Z

...resso-support/src/main/java/com/apollographql/apollo/test/espresso/ApolloIdlingResource.java

+/**
+ * An Espresso {@link IdlingResource} for {@link com.apollographql.apollo.ApolloClient}.
+ */
+public class ApolloIdlingResource implements IdlingResource {


make final?

marwanad · 2017-05-23T19:53:41Z

apollo-runtime/src/main/java/com/apollographql/apollo/internal/ApolloCallTracker.java

+ */
+public final class ApolloCallTracker {
+
+  private IdleResourceCallback idleResourceCallback;


Do we really need a class for that? Can't we just take a Runnable?

I would not use Runnable for this as this is really callback not some runnable action that we need to run.

VisheshVadhera · 2017-05-24T07:00:46Z

@marwanad Well that was the first thought which came to my mind to combine dispatcher and tracker in one class, but decided against it, because I wanted to keep the concerns separate and have a class whose only concern is to track calls and do nothing more.

sav007 · 2017-05-25T21:51:15Z

@VisheshVadhera I see all tests related to cache are failing could you pls fix that we can merge this PR

VisheshVadhera · 2017-05-26T05:00:32Z

On it.

…

On 26 May 2017 3:21 am, "Ivan Savytskyi" ***@***.***> wrote: @VisheshVadhera <https://github.com/visheshvadhera> I see all tests related to cache are failing could you pls fix that we can merge this PR — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#469 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD1DEMaZSeMQP_k9BI0LBCYrzMnBlXt7ks5r9ffUgaJpZM4NL6j-> .

… feature-espresso-idling-resource

Rename callTracker to tracker in ApolloClient

… feature-espresso-idling-resource

VisheshVadhera · 2017-05-30T18:03:33Z

@sav007 Fixed the failing tests. Feel free to merge this PR.

VisheshVadhera added 21 commits April 19, 2017 12:13

Add apollo-idling-resource module

7a1664d

Restructuring of the module

3ad4610

Add IdlingResource class

1210f32

Add dispatcher

0dda9b8

Add ApolloTracker for tracking the requests

51817a7

Execute callback on calls finished

76780b3

Register espresso callback with apolloTracker

0af44d1

Add basic test to check that idlingResource is being created properly

989e102

Add test for idleNow

b68e8e5

Add test for registerCallBack

ff6fa0d

Rename variables

7158335

Change variable names

5525529

Add tests for apolloCallTracker

43a90b6

Hide usage of apolloTracker in apolloClient

53801bf

Not exposing apolloTracker to the clients. Instead they have to call apolloClient's getRunningCount method and setIdleCallback method which then delegates these calls to the apolloTracker

Fix tests in apolloCallTracker

4483920

Add tests for syncCalls

190ae11

Add test for checking that idleCallback is invoked

664b033

Merge branch 'master' of github.com:apollographql/apollo-android into…

41dcb1d

… feature-espresso-idling-resource

Cleanup

72484ae

Add docs for ApolloCallTracker

cbf6574

Checkstyle

de9a829

VisheshVadhera added 2 commits May 2, 2017 11:02

Merge branch 'master' of github.com:apollographql/apollo-android into…

1cc6187

… feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java

Merge branch 'feature-espresso-idling-resource' of github.com:Vishesh…

07d4b91

…Vadhera/apollo-android into feature-espresso-idling-resource

VisheshVadhera mentioned this pull request May 2, 2017

Propagate CacheKeyResolver changes back to the apollo-sample module #477

Closed

Temp fix for apollo-sample module

5d23a9f

Revert the change made in last commit

69a28d5

VisheshVadhera added 5 commits May 12, 2017 12:47

Merge branch 'feature-espresso-idling-resource' of github.com:Vishesh…

1860193

…Vadhera/apollo-android into feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/internal/ApolloCallTracker.java

Fix checkstyle

f47672b

Fix checkstyle

c8102ba

Fix failing test

dd3cb51

Merge branch 'master' of github.com:apollographql/apollo-android into…

95c9383

… feature-espresso-idling-resource Conflicts: gradle/dependencies.gradle

sav007 reviewed May 13, 2017

View reviewed changes

sav007 approved these changes May 13, 2017

View reviewed changes

sav007 mentioned this pull request May 13, 2017

Fix package name prefix #481

Merged

VisheshVadhera added 2 commits May 14, 2017 02:01

Change package & module name

f9d36a8

Merge branch 'master' of github.com:apollographql/apollo-android into…

ac1e4a8

… feature-espresso-idling-resource

Merge branch 'master' of github.com:apollographql/apollo-android into…

516615b

… feature-espresso-idling-resource Conflicts: apollo-runtime/src/main/java/com/apollographql/apollo/ApolloClient.java apollo-runtime/src/main/java/com/apollographql/apollo/internal/RealApolloCall.java

marwanad reviewed May 23, 2017

View reviewed changes

Code review

4fab454

VisheshVadhera added 5 commits May 30, 2017 00:18

Merge branch 'master' of github.com:apollographql/apollo-android into…

57c401d

… feature-espresso-idling-resource

Change the order of methods in the class

c4e5ab5

Add missing tracker to builder in RealApolloCall

3dad93e

Rename callTracker to tracker in ApolloClient

Fix tests in queryFetcher

9820c66

Merge branch 'master' of github.com:apollographql/apollo-android into…

2096424

… feature-espresso-idling-resource

sav007 approved these changes Jun 1, 2017

View reviewed changes

sav007 merged commit 5b28d8d into apollographql:master Jun 1, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Espresso idling resource for Apollo GraphQL client #469

Espresso idling resource for Apollo GraphQL client #469

VisheshVadhera commented Apr 28, 2017 •

edited

Loading

VisheshVadhera commented Apr 29, 2017

sav007 commented May 2, 2017

VisheshVadhera commented May 3, 2017

sav007 May 13, 2017

VisheshVadhera May 13, 2017

sav007 May 13, 2017

VisheshVadhera May 13, 2017

sav007 May 13, 2017

VisheshVadhera May 13, 2017

sav007 left a comment

VisheshVadhera commented May 13, 2017

VisheshVadhera commented May 13, 2017 •

edited

Loading

sav007 commented May 16, 2017

marwanad commented May 16, 2017 •

edited

Loading

marwanad left a comment

marwanad May 17, 2017

VisheshVadhera May 24, 2017 •

edited

Loading

marwanad May 17, 2017

marwanad May 23, 2017

VisheshVadhera May 24, 2017

marwanad May 23, 2017

VisheshVadhera May 24, 2017

marwanad May 23, 2017

marwanad May 23, 2017

sav007 May 24, 2017

VisheshVadhera commented May 24, 2017

sav007 commented May 25, 2017

VisheshVadhera commented May 26, 2017 via email

VisheshVadhera commented May 30, 2017

		@@ -0,0 +1,4 @@
		POM_ARTIFACT_ID=apollo-idling-resource
		POM_NAME=Apollo GraphQL Idling Resource

		@@ -0,0 +1 @@
		/build

Espresso idling resource for Apollo GraphQL client #469

Espresso idling resource for Apollo GraphQL client #469

Conversation

VisheshVadhera commented Apr 28, 2017 • edited Loading

VisheshVadhera commented Apr 29, 2017

sav007 commented May 2, 2017

VisheshVadhera commented May 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sav007 left a comment

Choose a reason for hiding this comment

VisheshVadhera commented May 13, 2017

VisheshVadhera commented May 13, 2017 • edited Loading

sav007 commented May 16, 2017

marwanad commented May 16, 2017 • edited Loading

marwanad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VisheshVadhera May 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VisheshVadhera commented May 24, 2017

sav007 commented May 25, 2017

VisheshVadhera commented May 26, 2017 via email

VisheshVadhera commented May 30, 2017

VisheshVadhera commented Apr 28, 2017 •

edited

Loading

VisheshVadhera commented May 13, 2017 •

edited

Loading

marwanad commented May 16, 2017 •

edited

Loading

VisheshVadhera May 24, 2017 •

edited

Loading