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

Add final type hint to tests.unittest. #15072

Merged
merged 13 commits into from
Feb 14, 2023
5 changes: 3 additions & 2 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -2913,7 +2913,8 @@ def test_get_rooms_with_nonlocal_user(self) -> None:
other_user_tok = self.login("user", "pass")
event_builder_factory = self.hs.get_event_builder_factory()
event_creation_handler = self.hs.get_event_creation_handler()
storage_controllers = self.hs.get_storage_controllers()
persistence = self.hs.get_storage_controllers().persistence
assert persistence is not None

# Create two rooms, one with a local user only and one with both a local
# and remote user.
Expand All @@ -2940,7 +2941,7 @@ def test_get_rooms_with_nonlocal_user(self) -> None:

context = self.get_success(unpersisted_context.persist(event))

self.get_success(storage_controllers.persistence.persist_event(event, context))
self.get_success(persistence.persist_event(event, context))

# Now get rooms
url = "/_synapse/admin/v1/users/@joiner:remote_hs/joined_rooms"
Expand Down
15 changes: 11 additions & 4 deletions tests/rest/admin/test_username_available.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Optional

from twisted.test.proto_helpers import MemoryReactor

import synapse.rest.admin
Expand All @@ -33,17 +35,22 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.register_user("admin", "pass", admin=True)
self.admin_user_tok = self.login("admin", "pass")

async def check_username(username: str) -> bool:
if username == "allowed":
return True
async def check_username(
localpart: str,
guest_access_token: Optional[str] = None,
assigned_user_id: Optional[str] = None,
inhibit_user_in_use_error: bool = False,
) -> None:
if localpart == "allowed":
return
raise SynapseError(
400,
"User ID already taken.",
errcode=Codes.USER_IN_USE,
)

handler = self.hs.get_registration_handler()
handler.check_username = check_username
handler.check_username = check_username # type: ignore[assignment]
Comment on lines -36 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

General comment: how well does mypy detect out-of-date mocks like these? If we changed the signature of check_username in synapse but not in tests, would we notice it here?

(Not worth blocking this PR on)

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to sometimes pick-up that the signature is wrong, I'm not sure how consistently it does it though.

(Note that this isn't a mock though.)


def test_username_available(self) -> None:
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/rest/client/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ async def post_json(
return {}

# Register a mock that will return the expected result depending on the remote.
self.hs.get_federation_http_client().post_json = Mock(side_effect=post_json)
self.hs.get_federation_http_client().post_json = Mock(side_effect=post_json) # type: ignore[assignment]

# Check that we've got the correct response from the client-side endpoint.
self._test_status(
Expand Down
4 changes: 2 additions & 2 deletions tests/rest/client/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ def test_add_filter_for_other_user(self) -> None:

def test_add_filter_non_local_user(self) -> None:
_is_mine = self.hs.is_mine
self.hs.is_mine = lambda target_user: False
self.hs.is_mine = lambda target_user: False # type: ignore[assignment]
channel = self.make_request(
"POST",
"/_matrix/client/r0/user/%s/filter" % (self.user_id),
self.EXAMPLE_FILTER_JSON,
)

self.hs.is_mine = _is_mine
self.hs.is_mine = _is_mine # type: ignore[assignment]
self.assertEqual(channel.code, 403)
self.assertEqual(channel.json_body["errcode"], Codes.FORBIDDEN)

Expand Down
10 changes: 5 additions & 5 deletions tests/rest/client/test_presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ class PresenceTestCase(unittest.HomeserverTestCase):

def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:

presence_handler = Mock(spec=PresenceHandler)
presence_handler.set_state.return_value = make_awaitable(None)
self.presence_handler = Mock(spec=PresenceHandler)
self.presence_handler.set_state.return_value = make_awaitable(None)

hs = self.setup_test_homeserver(
"red",
federation_http_client=None,
federation_client=Mock(),
presence_handler=presence_handler,
presence_handler=self.presence_handler,
)

return hs
Expand All @@ -61,7 +61,7 @@ def test_put_presence(self) -> None:
)

self.assertEqual(channel.code, HTTPStatus.OK)
self.assertEqual(self.hs.get_presence_handler().set_state.call_count, 1)
self.assertEqual(self.presence_handler.set_state.call_count, 1)

@unittest.override_config({"use_presence": False})
def test_put_presence_disabled(self) -> None:
Expand All @@ -76,4 +76,4 @@ def test_put_presence_disabled(self) -> None:
)

self.assertEqual(channel.code, HTTPStatus.OK)
self.assertEqual(self.hs.get_presence_handler().set_state.call_count, 0)
self.assertEqual(self.presence_handler.set_state.call_count, 0)
7 changes: 5 additions & 2 deletions tests/rest/client/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def test_POST_disabled_registration(self) -> None:
self.assertEqual(channel.json_body["errcode"], "M_FORBIDDEN")

def test_POST_guest_registration(self) -> None:
self.hs.config.key.macaroon_secret_key = "test"
self.hs.config.key.macaroon_secret_key = b"test"
self.hs.config.registration.allow_guest_access = True

channel = self.make_request(b"POST", self.url + b"?kind=guest", b"{}")
Expand Down Expand Up @@ -1166,12 +1166,15 @@ def test_background_job(self) -> None:
"""
user_id = self.register_user("kermit_delta", "user")

self.hs.config.account_validity.startup_job_max_delta = self.max_delta
self.hs.config.account_validity.account_validity_startup_job_max_delta = (
self.max_delta
)
Comment on lines -1169 to +1171
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this just broken before?

Copy link
Member Author

@clokep clokep Feb 14, 2023

Choose a reason for hiding this comment

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

I think so. (Which makes me wonder why we care...)


now_ms = self.hs.get_clock().time_msec()
self.get_success(self.store._set_expiration_date_when_missing())

res = self.get_success(self.store.get_expiration_ts_for_user(user_id))
assert res is not None

self.assertGreaterEqual(res, now_ms + self.validity_period - self.max_delta)
self.assertLessEqual(res, now_ms + self.validity_period)
Expand Down
6 changes: 4 additions & 2 deletions tests/rest/client/test_retention.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ def test_visibility(self) -> None:
# Send a first event, which should be filtered out at the end of the test.
resp = self.helper.send(room_id=room_id, body="1", tok=self.token)
first_event_id = resp.get("event_id")
assert isinstance(first_event_id, str)

# Advance the time by 2 days. We're using the default retention policy, therefore
# after this the first event will still be valid.
Expand All @@ -144,6 +145,7 @@ def test_visibility(self) -> None:
# Send another event, which shouldn't get filtered out.
resp = self.helper.send(room_id=room_id, body="2", tok=self.token)
valid_event_id = resp.get("event_id")
assert isinstance(valid_event_id, str)

# Advance the time by another 2 days. After this, the first event should be
# outdated but not the second one.
Expand Down Expand Up @@ -229,7 +231,7 @@ def _test_retention_event_purged(self, room_id: str, increment: float) -> None:

# Check that we can still access state events that were sent before the event that
# has been purged.
self.get_event(room_id, create_event.event_id)
self.get_event(room_id, bool(create_event))

def get_event(self, event_id: str, expect_none: bool = False) -> JsonDict:
event = self.get_success(self.store.get_event(event_id, allow_none=True))
Expand All @@ -238,7 +240,7 @@ def get_event(self, event_id: str, expect_none: bool = False) -> JsonDict:
self.assertIsNone(event)
return {}

self.assertIsNotNone(event)
assert event is not None

time_now = self.clock.time_msec()
serialized = self.serializer.serialize_event(event, time_now)
Expand Down
12 changes: 7 additions & 5 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3382,8 +3382,8 @@ def test_threepid_invite_spamcheck_deprecated(self) -> None:
# a remote IS. We keep the mock for make_and_store_3pid_invite around so we
# can check its call_count later on during the test.
make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0)))
self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock
self.hs.get_identity_handler().lookup_3pid = Mock(
self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock # type: ignore[assignment]
self.hs.get_identity_handler().lookup_3pid = Mock( # type: ignore[assignment]
return_value=make_awaitable(None),
)

Expand Down Expand Up @@ -3443,8 +3443,8 @@ def test_threepid_invite_spamcheck(self) -> None:
# a remote IS. We keep the mock for make_and_store_3pid_invite around so we
# can check its call_count later on during the test.
make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0)))
self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock
self.hs.get_identity_handler().lookup_3pid = Mock(
self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock # type: ignore[assignment]
self.hs.get_identity_handler().lookup_3pid = Mock( # type: ignore[assignment]
return_value=make_awaitable(None),
)

Expand Down Expand Up @@ -3563,8 +3563,10 @@ def _inject_outlier(self, room_id: str) -> EventBase:
)

event.internal_metadata.outlier = True
persistence = self._storage_controllers.persistence
assert persistence is not None
self.get_success(
self._storage_controllers.persistence.persist_event(
persistence.persist_event(
event, EventContext.for_outlier(self._storage_controllers)
)
)
Expand Down
6 changes: 4 additions & 2 deletions tests/rest/client/test_shadow_banned.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def test_invite(self) -> None:
def test_invite_3pid(self) -> None:
"""Ensure that a 3PID invite does not attempt to contact the identity server."""
identity_handler = self.hs.get_identity_handler()
identity_handler.lookup_3pid = Mock(
identity_handler.lookup_3pid = Mock( # type: ignore[assignment]
side_effect=AssertionError("This should not get called")
)

Expand Down Expand Up @@ -222,7 +222,7 @@ def test_typing(self) -> None:
event_source.get_new_events(
user=UserID.from_string(self.other_user_id),
from_key=0,
limit=None,
limit=10,
room_ids=[room_id],
is_guest=False,
)
Expand Down Expand Up @@ -286,6 +286,7 @@ def test_displayname(self) -> None:
self.banned_user_id,
)
)
assert event is not None
self.assertEqual(
event.content, {"membership": "join", "displayname": original_display_name}
)
Expand Down Expand Up @@ -321,6 +322,7 @@ def test_room_displayname(self) -> None:
self.banned_user_id,
)
)
assert event is not None
self.assertEqual(
event.content, {"membership": "join", "displayname": original_display_name}
)
2 changes: 1 addition & 1 deletion tests/rest/client/test_upgrade_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def test_upgrade(self) -> None:
self.room_id, EventTypes.Tombstone, ""
)
)
self.assertIsNotNone(tombstone_event)
assert tombstone_event is not None
self.assertEqual(new_room_id, tombstone_event.content["replacement_room"])

# Check that the new room exists.
Expand Down