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

Add Mocking to Unit tests so they don't make For Realz api calls #31

Closed
missaugustina opened this issue Apr 12, 2018 · 4 comments
Closed
Assignees

Comments

@missaugustina
Copy link
Member

The unit tests are currently making "IRL" api calls which can result in failures in the event of no network access. Additionally, these tests may result in errors beyond the scope of what is actually being tested. Ultimately, at least for these tests, we just want to know if our library code is working, we don't care about the api key or httr.

That said, other end-to-end continuous integration tests should be developed that do call the API from TravisCi but that's beyond the scope of this issue. I'll file it as a second one and mention this one to link it.

@missaugustina missaugustina self-assigned this Apr 12, 2018
@missaugustina
Copy link
Member Author

missaugustina commented Apr 12, 2018

I found a package called "mockery" is this the appropriate one to use or do people have other ones they prefer?

Looks like testthat() has a "with_mock" function - https://www.rdocumentation.org/packages/testthat/versions/2.0.0/topics/with_mock

missaugustina added a commit that referenced this issue Apr 12, 2018
 * use with_mock to override GET()
 * create a testdata folder for storing mocks
 * improve tests for find_groups.R
 * add a test for internals.R

Part of Issue #31
missaugustina added a commit that referenced this issue Apr 12, 2018
 * use with_mock to override GET()
 * create a testdata folder for storing mocks
 * improve tests for find_groups.R
 * add a test for internals.R

Part of Issue #31
missaugustina added a commit that referenced this issue Apr 12, 2018
missaugustina added a commit that referenced this issue Apr 12, 2018
 * get_events now tests the get_events call
 * added TODO's for future tests based on logical branches
 * find_groups now runs with and without optional params

Part of Issue #31
@ledell
Copy link
Member

ledell commented Apr 12, 2018

@missaugustina So these tests are in addition to the real ones, or are you proposing that we replace the live tests with mock tests? It looks like you have opened another issue to add live tests back in? #33

missaugustina added a commit that referenced this issue Apr 13, 2018
@missaugustina
Copy link
Member Author

@ledell yes, I'm proposing that the tests that are currently live be replaced with the mocks. The existing tests are disabled in TravisCI and they failed for me when I ran them locally.

And yes, I'm proposing in #33 that we add integration tests to make sure our stuff still works with the Meetup API. I would see these just running when a PR was merged and shouldn't be too fragile.

Does that sound ok? Or would you rather I did the work on #33 before this one could be merged?

Note that this one enables the tests on TravisCI, so we get test coverage with every change TravisCI builds.

@missaugustina
Copy link
Member Author

Looks like my change got merged! We should make sure we get some tests in per #33 to monitor meetup api compatibility.

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

No branches or pull requests

2 participants