Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

annotate tests.server.FakeChannel #13136

Merged
merged 5 commits into from
Jul 4, 2022
Merged

Conversation

DMRobertson
Copy link
Contributor

and fix up tests that are inspecting json_body.

I chose to annotate this as Dict[str, Any] (although it could in
principle be any json payload: str, number, list, boolean, null).

Mypy now knows that json_body.get("foo") returns Optional[Any] and is
complaining in a few places. I generally replaced these with direct
lookups, since either

  • the test would already fail if the key was missing, or
  • the test probably ought to fail if the key was missing.

Happy to be persuaded otherwise if you're not convinced.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

David Robertson added 3 commits June 30, 2022 00:42
I chose to annotate this as `Dict[str, Any]` (although it could in
principle be any json payload: str, number, list, boolean, null).

Mypy now knows that json_body.get("foo") returns `Optional[Any]` and is
complaining in a few places. I generally replaced these with direct
lookups, since either
- the test would already fail if the key was missing, or
- the test probably ought to fail if the key was missing.

Happy to be persuaded otherwise if you're not convinced.
@DMRobertson DMRobertson marked this pull request as ready for review June 29, 2022 23:46
@DMRobertson DMRobertson requested a review from a team as a code owner June 29, 2022 23:46
@@ -1488,7 +1488,7 @@ def test_create_user_mau_limit_reached_active_admin(self) -> None:

if channel.code != HTTPStatus.OK:
raise HttpResponseException(
channel.code, channel.result["reason"], channel.json_body
channel.code, channel.result["reason"], channel.result["body"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we changing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think I just picked this up during a find-and-replace.

Copy link
Contributor Author

@DMRobertson DMRobertson Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, without this change:

tests/rest/admin/test_user.py:1491: error: Argument 3 to "HttpResponseException" has incompatible type "Dict[str, Any]"; expected "bytes"  [arg-type]

@DMRobertson DMRobertson merged commit d102ad6 into develop Jul 4, 2022
@DMRobertson DMRobertson deleted the dmr/typing/tests-fakechannel branch July 4, 2022 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants