|
| 1 | +Writing Tests for github3.py |
| 2 | +============================ |
| 3 | + |
| 4 | +Writing tests for github3.py is a non-trivial task and takes some |
| 5 | +understanding of the expecter_ module and mock_ modules. |
| 6 | + |
| 7 | +expecter |
| 8 | +-------- |
| 9 | + |
| 10 | +I chose to use the expecter_ module in the tests for github3.py because I feel |
| 11 | +that writing tests with it read far more naturally. Take for instance the |
| 12 | +following: |
| 13 | + |
| 14 | +.. code-block:: python |
| 15 | +
|
| 16 | + x = 1 |
| 17 | +
|
| 18 | + def foo(x): |
| 19 | + return [x + 1] |
| 20 | +
|
| 21 | + self.assertEquals(foo(x), [x + 1]) |
| 22 | +
|
| 23 | +You have the variable ``x``, the function ``foo`` and the inherited |
| 24 | +``assertEquals`` method from ``unittest.TestCase``. My first issue with this |
| 25 | +is that the unittest module, although technically grandfathered in, uses |
| 26 | +camel-casing for its methods. To me, this is ugly and annoying to type. I've |
| 27 | +found myself typing ``self.assertequals`` more often than not. Now consider |
| 28 | +this: |
| 29 | + |
| 30 | +.. code-block:: python |
| 31 | +
|
| 32 | + x = 1 |
| 33 | +
|
| 34 | + def foo(x): |
| 35 | + return [x + 1] |
| 36 | +
|
| 37 | + expect(foo(x)) == [x + 1] |
| 38 | +
|
| 39 | +This tests the exact same thing but this reads differently. Instead of "assert |
| 40 | +that ``foo(x)`` and ``[x + 1]`` are equal", you're saying "expect ``foo(x)`` |
| 41 | +and ``[x + 1]`` to be equal". It's the same thing, but the latter reads better |
| 42 | +than the former to me. |
| 43 | + |
| 44 | +The standard ``expecter`` module has fewer methods than what we use in |
| 45 | +github3.py but that's because I've subclassed the ``expect`` class and renamed |
| 46 | +it to look the same in github3.py. So the extra functionality are the |
| 47 | +following methods: |
| 48 | + |
| 49 | +- ``is_not_None`` which expects the object passed in to be anything but |
| 50 | + ``None``. Example usage:: |
| 51 | + |
| 52 | + expect(a).is_not_None() |
| 53 | + |
| 54 | +- ``is_None`` which expects the opposite of ``is_not_None``. Example usage:: |
| 55 | + |
| 56 | + expect(None).is_None() |
| 57 | + |
| 58 | +- ``is_True`` which expects the object passed in to be True. Example usage:: |
| 59 | + |
| 60 | + expect(True).is_True() |
| 61 | + |
| 62 | +- ``is_False`` which expects the object passed in to be False. Example usage:: |
| 63 | + |
| 64 | + expect(False).is_False() |
| 65 | + |
| 66 | +- ``is_in(iterable)`` which expects the object passed in be in ``iterable``. |
| 67 | + Example usage:: |
| 68 | + |
| 69 | + expect('foo').is_in(['foo', 'bar', 'bogus']) |
| 70 | + |
| 71 | +- ``githuberror`` which is used as a context manager to show that we're |
| 72 | + expecting something to raise a ``GitHubError``. Example usage:: |
| 73 | + |
| 74 | + with expect.githuberror(): |
| 75 | + github3.authorize() |
| 76 | + |
| 77 | +Why implement these custom methods? |
| 78 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 79 | + |
| 80 | +When using ``expecter.expect``, you receive an instance of a class instead of |
| 81 | +the actual object, so where you would like the following to work, it does not: |
| 82 | + |
| 83 | +.. code-block:: python |
| 84 | +
|
| 85 | + expect(a) is not None |
| 86 | +
|
| 87 | +The reality, however, is that test is tautologically true. Every instance of |
| 88 | +``expecter.expect`` is not ``None``. In other cases, tools like ``pep8`` and |
| 89 | +``flake8`` will complain if you do: |
| 90 | + |
| 91 | +.. code-block:: python |
| 92 | +
|
| 93 | + expect(a) == True |
| 94 | +
|
| 95 | +And they rightfully complain. So these methods just make life easier. If they |
| 96 | +cause significant enough confusion, then I'll consider rewriting the test |
| 97 | +suite in something better. |
| 98 | + |
| 99 | +mock |
| 100 | +---- |
| 101 | + |
| 102 | +The mock library written by Michael Foord is a fantastic tool and is entirely |
| 103 | +necessary to testing github3.py. Last year, GitHub changed their ratelimit |
| 104 | +(for anonymous requests) from 5000 per hour to 60 per hour. This meant that |
| 105 | +all of a sudden, github3.py's tests failed and failed miserably when trying to |
| 106 | +test directly against the API. The best solution was to collect all of the |
| 107 | +possible JSON responses and store then locally. You can find them in |
| 108 | +``tests/json/``. We then had to construct our own fake ``requests.Response`` |
| 109 | +objects and mock the ``request`` method on ``requests.Session`` objects. To |
| 110 | +help do this, I wrote some methods that are present on the ``BaseCase`` class: |
| 111 | + |
| 112 | +- ``response`` takes the name of the file in ``tests/json``, the |
| 113 | + ``status_code``, the "default" encoding for the data, optional headers and a |
| 114 | + paramtere ``_iter`` which determines if the results should be iterable or |
| 115 | + not. This then constructs a ``requests.Response`` object and sets it as the |
| 116 | + return value of the mocked ``requests.Session#request`` method. |
| 117 | + |
| 118 | +- ``get``, ``put``, ``patch``, ``post``, ``delete`` all modify a tuple that |
| 119 | + looks like: ``(METHOD, url)`` where ``METHOD`` is either ``GET``, ``PUT``, |
| 120 | + &c. and the ``url`` is passed to the method. |
| 121 | + |
| 122 | +- ``mock_assertions`` has a set of assertions it makes about **every** request |
| 123 | + we deal with and which are true of every request to the API. After making |
| 124 | + these assertions, it resets the mock in case it needs to be used again |
| 125 | + during the same test. |
| 126 | + |
| 127 | +- ``not_called`` asserts that at no point was the mock used up until this |
| 128 | + point. |
| 129 | + |
| 130 | +The ``setUp`` and ``tearDown`` methods take care of instantiating the mock |
| 131 | +object that we use in this case. The code for those methods are taken directly |
| 132 | +from mocks documentation. |
| 133 | + |
| 134 | +Walking through a couple real tests |
| 135 | +----------------------------------- |
| 136 | + |
| 137 | +Simple |
| 138 | +~~~~~~ |
| 139 | + |
| 140 | +From ``tests/test_gists.py``: |
| 141 | + |
| 142 | +.. code-block:: python |
| 143 | +
|
| 144 | + def test_unstar(self): |
| 145 | + self.response('', 204) |
| 146 | + self.delete(self.api) |
| 147 | + self.conf = {} |
| 148 | +
|
| 149 | + with expect.githuberror(): |
| 150 | + self.gist.unstar() |
| 151 | +
|
| 152 | + self.not_called() |
| 153 | + self.login() |
| 154 | + expect(self.gist.unstar()).is_True() |
| 155 | + self.mock_assertions() |
| 156 | +
|
| 157 | +First notice that this, like every other test, is prefaced with ``test_`` and |
| 158 | +then followed by the name of the method it is testing, in this case, |
| 159 | +``unstar``. |
| 160 | + |
| 161 | +The first thing we then do is call ``self.response('', 204)`` which means |
| 162 | +we're going to be mocking a response with No Content and a status code of 204. |
| 163 | +Then we cal ``self.delete(self.api)``. ``self.api`` is an attribute I've set |
| 164 | +on this class which has the URL that will be used to communicate with the |
| 165 | +GitHub API 90% of the time. (Other times it may be modified.) ``self.delete`` |
| 166 | +simply sets ``self.args = ('DELETE', self.api)``. Then we use one of our |
| 167 | +custom expect methods. Right now, the ``Gist`` object stored in ``self.gist`` |
| 168 | +thinks the user is anonymous so calling ``unstar`` on it should raise a |
| 169 | +``GitHubError``. If it didn't, expect would raise an ``AssertionError`` |
| 170 | +exception and the test would fail. If that does not happen, then we just check |
| 171 | +(because we're paranoid) that the mock was not called with |
| 172 | +``self.not_called``. Next we login, and assert that calling ``unstar`` results |
| 173 | +in ``True``. Finally, we make sure those core assertions about the mock held. |
| 174 | + |
| 175 | +Moderate |
| 176 | +~~~~~~~~ |
| 177 | + |
| 178 | +From ``tests/test_gists.py``: |
| 179 | + |
| 180 | +.. code-block:: python |
| 181 | +
|
| 182 | + def test_create_comment(self): |
| 183 | + self.response('gist_comment', 201) |
| 184 | + self.post(self.api + '/comments') |
| 185 | + self.conf = {'data': {'body': 'bar'}} |
| 186 | +
|
| 187 | + with expect.githuberror(): |
| 188 | + self.gist.create_comment(None) |
| 189 | +
|
| 190 | + self.login() |
| 191 | +
|
| 192 | + expect(self.gist.create_comment(None)).is_None() |
| 193 | + expect(self.gist.create_comment('')).is_None() |
| 194 | + self.not_called() |
| 195 | + expect(self.gist.create_comment('bar')).isinstance( |
| 196 | + gists.comment.GistComment) |
| 197 | + self.mock_assertions() |
| 198 | +
|
| 199 | +Now we're setting an attribute called ``conf`` with ``{'data': {'body': |
| 200 | +'bar'}}``. We use this to assert that the data we're sending to GitHub is |
| 201 | +actually sent. |
| 202 | + |
| 203 | +You'll now see that there are two calls to ``create_comment`` where we expect |
| 204 | +to receive ``None`` because github3.py refused to act on bad data. We then |
| 205 | +make sure that nothing was called and create a comment with the text ``'bar'`` |
| 206 | +and expect it to return an instance of ``GistComment``. Notice how the |
| 207 | +**body** of the new comment is **bar**. |
| 208 | + |
| 209 | +Difficult |
| 210 | +~~~~~~~~~ |
| 211 | + |
| 212 | +From ``tests/test_repos.py``: |
| 213 | + |
| 214 | +.. code-block:: python |
| 215 | +
|
| 216 | + def test_archive(self): |
| 217 | + headers = {'content-disposition': 'filename=foo'} |
| 218 | + self.response('archive', 200, **headers) #** |
| 219 | + self.get(self.api + 'tarball/master') |
| 220 | + self.conf.update({'stream': True}) |
| 221 | +
|
| 222 | + expect(self.repo.archive(None)).is_False() |
| 223 | +
|
| 224 | + expect(os.path.isfile('foo')).is_False() |
| 225 | + expect(self.repo.archive('tarball')).is_True() |
| 226 | + expect(os.path.isfile('foo')).is_True() |
| 227 | + os.unlink('foo') |
| 228 | + self.mock_assertions() |
| 229 | +
|
| 230 | + self.request.return_value.raw.seek(0) |
| 231 | + self.request.return_value._content_consumed = False |
| 232 | +
|
| 233 | + expect(os.path.isfile('path_to_file')).is_False() |
| 234 | + expect(self.repo.archive('tarball', 'path_to_file')).is_True() |
| 235 | + expect(os.path.isfile('path_to_file')).is_True() |
| 236 | + os.unlink('path_to_file') |
| 237 | +
|
| 238 | + self.request.return_value.raw.seek(0) |
| 239 | + self.request.return_value._content_consumed = False |
| 240 | +
|
| 241 | + self.get(self.api + 'zipball/randomref') |
| 242 | + expect(self.repo.archive('zipball', ref='randomref')).is_True() |
| 243 | + os.unlink('foo') |
| 244 | +
|
| 245 | + self.request.return_value.raw.seek(0) |
| 246 | + self.request.return_value._content_consumed = False |
| 247 | +
|
| 248 | + o = mock_open() |
| 249 | + with patch('{0}.open'.format(__name__), o, create=True): |
| 250 | + with open('archive', 'wb+') as fd: |
| 251 | + self.repo.archive('tarball', fd) |
| 252 | +
|
| 253 | + o.assert_called_once_with('archive', 'wb+') |
| 254 | + fd = o() |
| 255 | + fd.write.assert_called_once_with(b'archive_data') |
| 256 | +
|
| 257 | +We start this test by setting up headers that are set by GitHub when returning |
| 258 | +data like an archive. We then pass those headers to the Response constructor |
| 259 | +and set the url. We're also expecting that github3.py is going to pass |
| 260 | +``stream=True`` to the request. We then finally make a request and test the |
| 261 | +assertions about the mock. That resets the mock and then we can go on to test |
| 262 | +the other features of the ``archive`` method. At the end, we mock the built-in |
| 263 | +``open`` method, but that's covered in the mock documentation. |
| 264 | + |
| 265 | +.. _expecter: http://expecter-gadget.rtfd.org |
| 266 | +.. _mock: http://mock.rtfd.org |
0 commit comments