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

Unit tests for API endpoints #396

Closed
miketaylr opened this issue Nov 14, 2014 · 18 comments
Closed

Unit tests for API endpoints #396

miketaylr opened this issue Nov 14, 2014 · 18 comments

Comments

@miketaylr
Copy link
Member

We could write some tests around our API endpoints in a similar style to the "URL" unit tests that we currently have, to be run with the nosetests command. When we have Travis running, we'll have it run both functional Selenium tests as well as our nosetests.

Logged out state will be easiest, perhaps a good first step.

@karlcow would you like to help out with this? I bet you could sniff out more than a few bugs.

@miketaylr
Copy link
Member Author

We probably want to mock JSON responses from GitHub. https://github.com/dropbox/responses looks cool. I'm sure there's lots more out there.

@karlcow
Copy link
Member

karlcow commented Nov 14, 2014

Yes. That's a very good idea and I'll also try to document at the same time (see #397).

@miketaylr
Copy link
Member Author

I added a stub for this as part of #420 (33eb430). Seemed good to add some tests as we add a new endpoint.

@karlcow
Copy link
Member

karlcow commented Aug 19, 2015

In #667 I tested all the error issues. Once the PR is merged they will
appear as https://github.com/webcompat/webcompat.com/blob/master/tests/test_api_urls.py

I guess I can add tests for success.

@karlcow
Copy link
Member

karlcow commented Feb 11, 2016

ok important comment for this issue. I need to describe this at a point. It took me a lot of time to pull out the right strings.
#712 (comment)

There's a partial documentation here but which is "hidden" in the functional tests.
https://github.com/webcompat/webcompat.com/blob/master/CONTRIBUTING.md#functional-tests-using-fixture-data
Maybe we should put it higher up for the general principle and then details how to write it for functional tests and for unit tests.

ok. Starting to have a clearer picture.

@karlcow
Copy link
Member

karlcow commented Feb 15, 2016

Experimenting

diff --git a/tests/test_api_urls.py b/tests/test_api_urls.py
index cb0c9c9..4d6f887 100644
--- a/tests/test_api_urls.py
+++ b/tests/test_api_urls.py
@@ -27,7 +27,7 @@ class TestAPIURLs(unittest.TestCase):
     def setUp(self):
         # Switch to False here because we don't want to send the mocked
         # Fixture data. Which is OK because these don't touch GitHub API data.
-        webcompat.app.config['TESTING'] = False
+        webcompat.app.config['TESTING'] = True
         self.app = webcompat.app.test_client()

     def tearDown(self):

Then nosetests -v tests/test_api_urls.py gives me

API issue for a non existent number returns JSON 404. ... ERROR
API issue search with bad keywords returns JSON 404. ... ok
API access to labels without auth returns JSON 200. ... ERROR
API with wrong parameter returns JSON 404. ... ERROR
API setting labels without auth returns JSON 403 error code. ... ok
API access to user activity without auth returns JSON 401. ... ok
API with wrong category returns JSON 404. ... FAIL
API with wrong route returns JSON 404. ... ok

======================================================================
ERROR: API issue for a non existent number returns JSON 404.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karl/code/webcompat.com/tests/test_api_urls.py", line 45, in test_api_issues_out_of_range
    rv = self.app.get('/api/issues/1000000', environ_base=headers)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 774, in get
    return self.open(*args, **kw)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/testing.py", line 108, in open
    follow_redirects=follow_redirects)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 742, in open
    response = self.run_wsgi_app(environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 659, in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 867, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/karl/code/webcompat.com/webcompat/helpers.py", line 359, in wrapped_func
    with open(file_path + '.json', 'r') as f:
IOError: [Errno 2] No such file or directory: u'/Users/karl/code/webcompat.com/tests/fixtures/api/issues/1000000.json'

======================================================================
ERROR: API access to labels without auth returns JSON 200.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karl/code/webcompat.com/tests/test_api_urls.py", line 69, in test_api_labels_without_auth
    rv = self.app.get('/api/issues/labels', environ_base=headers)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 774, in get
    return self.open(*args, **kw)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/testing.py", line 108, in open
    follow_redirects=follow_redirects)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 742, in open
    response = self.run_wsgi_app(environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 659, in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 867, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/karl/code/webcompat.com/webcompat/helpers.py", line 359, in wrapped_func
    with open(file_path + '.json', 'r') as f:
IOError: [Errno 2] No such file or directory: u'/Users/karl/code/webcompat.com/tests/fixtures/api/issues/labels.json'

======================================================================
ERROR: API with wrong parameter returns JSON 404.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karl/code/webcompat.com/tests/test_api_urls.py", line 90, in test_api_search_wrong_parameter
    rv = self.app.get('/api/issues/search?z=foobar', environ_base=headers)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 774, in get
    return self.open(*args, **kw)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/testing.py", line 108, in open
    follow_redirects=follow_redirects)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 742, in open
    response = self.run_wsgi_app(environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 659, in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/test.py", line 867, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/karl/code/webcompat.com/webcompat/helpers.py", line 359, in wrapped_func
    with open(file_path + '.json', 'r') as f:
IOError: [Errno 2] No such file or directory: u'/Users/karl/code/webcompat.com/tests/fixtures/api/issues/search.c26700a387892b7362cbbd64daf39f73.json'
-------------------- >> begin captured stdout << ---------------------
Expected fixture file: /Users/karl/code/webcompat.com/tests/fixtures/api/issues/search.c26700a387892b7362cbbd64daf39f73.json

--------------------- >> end captured stdout << ----------------------

======================================================================
FAIL: API with wrong category returns JSON 404.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karl/code/webcompat.com/tests/test_api_urls.py", line 63, in test_api_wrong_category
    self.assertEqual(rv.status_code, 404)
AssertionError: 200 != 404

----------------------------------------------------------------------
Ran 8 tests in 0.025s

FAILED (errors=3, failures=1)

I can see two options:

  1. Create new fixtures for these failing tests. (Interesting I guess we were hitting the network somehow for these.)
  2. Separate the tests which clearly requires network call from the others.

Summary of the failure:

  • ERROR: API issue for a non existent number returns JSON 404.
    This could be mitigated if we were keeping in memory (webcompat side) the last issue number created. so before calling GitHub we could always check if it's higher than the highest issue number. The test would then to see if we return the 404 when we request the highest issue number + 100 (just to be secure).
  • ERROR: API access to labels without auth returns JSON 200.
    This test was added by @hallvors. There is no fixture for it currently. IOError: [Errno 2] No such file or directory: u'/Users/karl/code/webcompat.com/tests/fixtures/api/issues/labels.json'
  • ERROR: API with wrong parameter returns JSON 404.
    Here again it could be tighter. by modifying the way we handle our input. We could decompose the URI and only allow the parameters we wish to have on this URI, and bails out if it's not one of them. That would avoid the call to GitHub, remove the need for a fixture and tighten the API.
  • FAIL: API with wrong category returns JSON 404.
    Here we really have an issue, because it should just return a 404 and it's not happening. Other question, with TESTING set to False, this test doesn't fail, which makes me suspicious about the whole thing. AssertionError: 200 != 404 Basically it returned a 200 instead of a 404. It should not have. Deep dive needed in https://github.com/webcompat/webcompat.com/blob/master/webcompat/api/endpoints.py#L98

@karlcow
Copy link
Member

karlcow commented Feb 15, 2016

Let's see, the simplest seems to be:

ERROR: API with wrong parameter returns JSON 404
https://github.com/webcompat/webcompat.com/blob/master/webcompat/api/endpoints.py#L143

With http HEAD 'http://localhost:5000/api/issues/search?z=foobar'
params returns MultiDict([('z', u'foobar')])
query_string returns None

which triggers abort(404). So we already take care of the invalid parameter!

The issue with the test is that it triggers an HTML response instead of a JSON response.

HTTP/1.0 404 NOT FOUND
Content-Length: 5784
Content-Type: text/html; charset=utf-8
Date: Mon, 15 Feb 2016 05:27:25 GMT
Server: Werkzeug/0.10.4 Python/2.7.10

hmmm I have the feeling that the mockable_response is somewhat screwing up the test. ping @miketaylr
If I remove the @mockable_response. My test is working again.

@karlcow
Copy link
Member

karlcow commented Feb 15, 2016

ok understood it's really the @mockable_response which screws things up here for @api.route('/issues/search')
https://github.com/webcompat/webcompat.com/blob/master/webcompat/api/endpoints.py#L143

The fixture is trying to send what "webcompat.com" would reply to such a request. I have the feeling it is not the fixture we want. The fixtures should be here to avoid the calls to GitHub.
We still want the calls to webcompat.com which is working locally.

So basically, we need to mock proxy_request and api_request.

@miketaylr
Copy link
Member Author

So basically, we need to mock proxy_request and api_request.

I think you're right.

I don't recall 100% why I didn't take this route initially -- I think it had to do something with the fact that proxy_request and api_request are very generic.

I wasn't sure how to get to the right fixture data from the method call without adding 20 if/else statements based on args into the method. But I bet you can think of a better way. This would be a very cool contribution. 💐

(and maybe we just define a decorator or two that handles the ugly argument -> fixture handling and toss that in helpers.py and we're done).

@karlcow
Copy link
Member

karlcow commented Feb 15, 2016

Yes it's completely logical what you did and a lot easier.

I was reading about MagicMock yesterday and I'll have to try to really understand. I was trying to understand if the side effects feature is exactly what you are mentioning about the specific cases in front of the generic mocking object.

To explore. Still I want to sit ⑁ with you in a café ☕️ and go on pair 👬 reading the code. Ah the future of transportation… 🚀

@miketaylr
Copy link
Member Author

https://blog.fugue.co/2016-02-11-python-mocking-101.html looks interesting, will take a look at some point soon.

(brb, finding a rocketship)

@karlcow
Copy link
Member

karlcow commented Feb 16, 2016

That would be a reason to move to 3.5.1
https://docs.python.org/3/library/unittest.mock.html#module-unittest.mock

Some ideas for prototyping:

in tests/ directory, we need a helper for reading the fixtures (the same way you did) or maybe patch is just enough and we don't need fixture.

We need variability on 3 parameters for the requests:

  • request_path
  • method (GET, POST)
  • Accept header

Depending on that the responses will be different.

  • Magicmock creates a mask a bit too tolerant, but there's an attribute for auto-speccing the mock.

Autospeccing is based on the existing spec feature of mock. It limits the api of mocks to the api of an original object (the spec), but it is recursive (implemented lazily) so that attributes of mocks only have the same api as the attributes of the spec. In addition mocked functions / methods have the same call signature as the original so they raise a TypeError if they are called incorrectly.

https://docs.python.org/3/library/unittest.mock.html#auto-speccing

Basically it seems it will copy the interface of the class you are mocking. So I guess in our case either Github Flask or requests modules.

@karlcow
Copy link
Member

karlcow commented Feb 16, 2016

Another useful article http://engineroom.trackmaven.com/blog/real-life-mocking/

@karlcow
Copy link
Member

karlcow commented Feb 16, 2016

In the test file, we can also do things like

from mock import patch, Mock

@patch.dict(webcompat.app.config, {'TESTING': 'True'})
def test_api_issues_search(self):
    '''API issue search with bad keywords returns JSON 404.'''
    rv = self.app.get('/api/issues/search/foobar', environ_base=headers)
    self.assertEqual(rv.status_code, 404)
    self.assertEqual(rv.content_type, 'application/json')

And here things with fixtures examples:
https://github.com/PyBossa/pybossa/blob/0239f545588204de8422430d8ff38d73c1360b60/test/test_s3_client.py

@karlcow
Copy link
Member

karlcow commented Feb 16, 2016

There is a backport of unittest.mock for earlier versions of Python, available as mock on PyPI.
https://pypi.python.org/pypi/mock

@miketaylr
Copy link
Member Author

from mock import patch, Mock

Ah, this is very cool.

@miketaylr
Copy link
Member Author

Status update: we want to do this, but need to be able to mock comms with GitHub first (see also #712).

@karlcow
Copy link
Member

karlcow commented Aug 24, 2017

Closing through the work of @brizental on #1618. Congrats!

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

No branches or pull requests

3 participants