zulip: Add tests for API functions. #682
Conversation
LoopThrough-i-j
force-pushed
the
test-suite
branch
from
ecf48a1
to
076da5f
Compare
zulip/setup.py
Outdated
| @@ -65,6 +65,7 @@ def recur_expand(target_root: Any, dir: Any) -> Generator[Tuple[str, List[str]], | |||
|
|
|||
| setuptools_info = dict( | |||
| install_requires=['requests[security]>=0.12.1', | |||
| 'responses', | |||
There was a problem hiding this comment.
We should make this a development-only dependency, like mypy.
zulip/tests/test_api_functions.py
Outdated
| url=re.compile("{}v1/messages/([0-9]*)/reactions".format(self.base_url)), | ||
| json={'result': 'success', 'msg': ''}, | ||
| status=200 | ||
| ) |
There was a problem hiding this comment.
I think we want to do this individually in each test, for just the endpoints we expect that test to call.
Additionally, I would configure it for the precise URL that the test is going to use, and assert what parameters were actually present in the HTTP request -- the goal here is to use this to verify that the API bindings made the precise request that we expected, not simulate a server (where the regular expression approach might be more appropriate).
LoopThrough-i-j
force-pushed
the
test-suite
branch
3 times, most recently
from
9c19d36
to
aca98cc
Compare
LoopThrough-i-j
force-pushed
the
test-suite
branch
2 times, most recently
from
e9633ac
to
97ab5c6
Compare
Value of type "Optional[Any]" is not indexable error was originated in PR zulip#682.
Value of type "Optional[Any]" is not indexable error was originated in PR zulip#682. This is due to request in Flask 2.0.
Value of type "Optional[Any]" is not indexable error was originated in PR #682. This is due to request in Flask 2.0.
|
I was thinking of simulating an event queue for testing |
This is an unit test suite testing for payloads and proper api calls.
LoopThrough-i-j
force-pushed
the
test-suite
branch
from
97ab5c6
to
781d0f3
Compare
|
@timabbott a review on this if possible would be great. |
| key, value = param.split("=") | ||
| params[key] = urllib.parse.unquote(value) | ||
| assert "emoji_name" in params or "emoji_code" in params | ||
| return (200, {}, json.dumps({'result': 'success', 'msg': ''})) |
There was a problem hiding this comment.
I don't understand this function -- why do we need this parsing logic?
There was a problem hiding this comment.
This function needs some work as well as I understand. This test is primarily for testing whether the payload provided is properly parsed, the URL is correct, and all the It accepts and sends a proper response, as a part of the normal API workflow.
This test makes sure all intermediate functions for an API call like do_api_query and call_endpoint, work correctly.
| self.assertEqual(result, {'result': 'success', 'msg': ''}) | ||
|
|
||
| @responses.activate | ||
| def test_call_on_each_event(self) -> None: |
There was a problem hiding this comment.
I think this test could benefit from a comment or docstring explaining what we're trying to test here.
|
@timabbott thanks for the review I will update the PR shortly, with required comments and explanations as best as I can. |
|
Heads up @LoopThrough-i-j, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
No description provided.