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

Removed the react-native test. #1451

Merged
merged 11 commits into from Dec 28, 2017

Conversation

Projects
None yet
2 participants
@excitement-engineer
Copy link
Collaborator

excitement-engineer commented Dec 23, 2017

This PR removes the only react-native test in the repo. I don’t really see the value of including this test in the repo, in addition it adds complexity with the jest configurations and adds some other devDependencies which can now be removed.

@excitement-engineer

This comment has been minimized.

Copy link
Collaborator Author

excitement-engineer commented Dec 23, 2017

Note, this is WIP. I am completing this as soon as I find some more time.

@excitement-engineer

This comment has been minimized.

Copy link
Collaborator Author

excitement-engineer commented Dec 27, 2017

I can confirm that the test will refetch active ObservableQuerys when resetting the client store in ObservableQuery.test.tsx is also broken on master. Because the test was implemented incorrectly (i.e. done was not used to stop the test`) it passed even though it is wrong.

What should we do with this test? It seems that this test has nothing to do with react-apollo but more how apollo-client works.

@excitement-engineer

This comment has been minimized.

Copy link
Collaborator Author

excitement-engineer commented Dec 27, 2017

@rosskevin

This comment has been minimized.

Copy link
Collaborator

rosskevin commented Dec 27, 2017

@excitement-engineer can you rebase - I'd like to get this one merged.

const globalAny: any = global;

globalAny.requestAnimationFrame = callback => {
setTimeout(callback, 0);

This comment has been minimized.

@rosskevin

rosskevin Dec 27, 2017

Collaborator

This is the long way to type it, you could just do (global as any).requestAnimationFrame

@excitement-engineer

This comment has been minimized.

Copy link
Collaborator Author

excitement-engineer commented Dec 27, 2017

It seems like the ci is not starting, maybe travis is down. The ci will not pass because of the broken test. Given that it seems to be testing something internal in apollo-client. Perhaps we should simply delete the test

@rosskevin

This comment has been minimized.

Copy link
Collaborator

rosskevin commented Dec 27, 2017

If you resolve conflicts and push a change, it will run.

excitement-engineer added some commits Dec 28, 2017

@excitement-engineer excitement-engineer merged commit 1c4ff31 into apollographql:master Dec 28, 2017

4 checks passed

CLA Author has signed the Meteor CLA.
Details
bundlesize ./lib/umd/react-apollo.js: 4.66KB < maxSize 4.7KB gzip (same as master)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 96.097%
Details

@excitement-engineer excitement-engineer deleted the excitement-engineer:removed-react-native-test branch Dec 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment