Add dummy and virtual objects for testing #125
Conversation
Current coverage is 84.97%@@ master #125 diff @@
==========================================
Files 36 36
Lines 1344 1344
Methods 0 0
Messages 0 0
Branches 123 123
==========================================
- Hits 1143 1142 -1
- Misses 165 166 +1
Partials 36 36
|
kitchoi
changed the title
|
@sbo I have added a number of dummy/virtual objects for Application, Accounting, Hub, ReverseProxy, ContainerManager and docker.Client I had to modify some tests to validate that these objects work. I also noticed that Some existing tests can switch to using these dummy objects, but I have not done in order to make it easier to review. |
|
Ok, the other branch touches more files, so to reduce risk let's merge that one first and this one second. We'll do the switch progressively or in a separate PR. In the meantime, I review this one and you can review the other one. |
| @@ -131,7 +131,7 @@ def test_retrieve(self): | |||
| self.assertEqual(res.code, httpstatus.NOT_FOUND) | |||
|
|
|||
| self._app.container_manager.docker_client.containers = \ | |||
| mock_coro_factory(return_value=[utils.containers_dict()]) | |||
| mock_coro_factory(return_value=[containers_dict()]) | |||
There was a problem hiding this comment.
I wonder if we can mock the internal sync client instead...
Just a comment. I would not do it now, but useful to consider. I changed the AsyncDockerClient.client to _sync_client, so if you change it now it would need change again. Let's wait.
|
General comment. A lot of the current boilerplate output we have in utils is now probably ready to disappear, which is good news |
|
|
||
|
|
||
| def create_reverse_proxy(params=None, | ||
| register_result='', |
There was a problem hiding this comment.
It would be convenient at this point to change mock_coro_factory to do exceptions as well as a return value, like mock does. It would simplify our general testing of exceptions.
There was a problem hiding this comment.
Again, don't work on that. It's good to keep it into account as a "small task". We can open individual issues for these.
|
ok. now there's a merge conflict as predicted. |
Conflicts: tests/docker/test_container_manager.py
|
@sbo And it is resolved! |
Provides for #98
Work in progress.
(should have named the branch differently...never mind)