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

Commit

Permalink
Remove redundant get_success calls in test code (#12346)
Browse files Browse the repository at this point in the history
There are a bunch of places we call get_success on an immediate value, which is unnecessary. Let's rip them out, and remove the redundant functionality in get_success and friends.
  • Loading branch information
richvdh authored Apr 1, 2022
1 parent c4cf916 commit 33ebee4
Show file tree
Hide file tree
Showing 13 changed files with 74 additions and 121 deletions.
1 change: 1 addition & 0 deletions changelog.d/12346.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove redundant `get_success` calls in test code.
25 changes: 12 additions & 13 deletions tests/handlers/test_deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,20 @@ def _deactivate_my_account(self) -> None:
Deactivates the account `self.user` using `self.token` and asserts
that it returns a 200 success code.
"""
req = self.get_success(
self.make_request(
"POST",
"account/deactivate",
{
"auth": {
"type": "m.login.password",
"user": self.user,
"password": "pass",
},
"erase": True,
req = self.make_request(
"POST",
"account/deactivate",
{
"auth": {
"type": "m.login.password",
"user": self.user,
"password": "pass",
},
access_token=self.token,
)
"erase": True,
},
access_token=self.token,
)

self.assertEqual(req.code, HTTPStatus.OK, req)

def test_global_account_data_deleted_upon_deactivation(self) -> None:
Expand Down
2 changes: 1 addition & 1 deletion tests/handlers/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.bob = UserID.from_string("@4567:test")
self.alice = UserID.from_string("@alice:remote")

self.get_success(self.register_user(self.frank.localpart, "frankpassword"))
self.register_user(self.frank.localpart, "frankpassword")

self.handler = hs.get_profile_handler()

Expand Down
4 changes: 1 addition & 3 deletions tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ def test_unknown_room_version(self):
)

# Blow away caches (supported room versions can only change due to a restart).
self.get_success(
self.store.get_rooms_for_user_with_stream_ordering.invalidate_all()
)
self.store.get_rooms_for_user_with_stream_ordering.invalidate_all()
self.store._get_event_cache.clear()

# The rooms should be excluded from the sync response.
Expand Down
20 changes: 9 additions & 11 deletions tests/module_api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,22 @@ def test_can_register_user(self):
self.assertEqual(displayname, "Bobberino")

def test_can_register_admin_user(self):
user_id = self.get_success(
self.register_user(
"bob_module_admin", "1234", displayname="Bobberino Admin", admin=True
)
user_id = self.register_user(
"bob_module_admin", "1234", displayname="Bobberino Admin", admin=True
)

found_user = self.get_success(self.module_api.get_userinfo_by_id(user_id))
self.assertEqual(found_user.user_id.to_string(), user_id)
self.assertIdentical(found_user.is_admin, True)

def test_can_set_admin(self):
user_id = self.get_success(
self.register_user(
"alice_wants_admin",
"1234",
displayname="Alice Powerhungry",
admin=False,
)
user_id = self.register_user(
"alice_wants_admin",
"1234",
displayname="Alice Powerhungry",
admin=False,
)

self.get_success(self.module_api.set_user_admin(user_id, True))
found_user = self.get_success(self.module_api.get_userinfo_by_id(user_id))
self.assertEqual(found_user.user_id.to_string(), user_id)
Expand Down
4 changes: 2 additions & 2 deletions tests/replication/slave/storage/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist(self):

event_source = RoomEventSource(self.hs)
event_source.store = self.slaved_store
current_token = self.get_success(event_source.get_current_key())
current_token = event_source.get_current_key()

# gradually stream out the replication
while repl_transport.buffer:
Expand All @@ -277,7 +277,7 @@ def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist(self):
self.pump(0)

prev_token = current_token
current_token = self.get_success(event_source.get_current_key())
current_token = event_source.get_current_key()

# attempt to replicate the behaviour of the sync handler.
#
Expand Down
12 changes: 3 additions & 9 deletions tests/rest/admin/test_server_notice.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,7 @@ def test_send_server_notice(self) -> None:
self.assertEqual(messages[0]["sender"], "@notices:test")

# invalidate cache of server notices room_ids
self.get_success(
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()
)
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()

# send second message
channel = self.make_request(
Expand Down Expand Up @@ -291,9 +289,7 @@ def test_send_server_notice_leave_room(self) -> None:
# invalidate cache of server notices room_ids
# if server tries to send to a cached room_id the user gets the message
# in old room
self.get_success(
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()
)
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()

# send second message
channel = self.make_request(
Expand Down Expand Up @@ -380,9 +376,7 @@ def test_send_server_notice_delete_room(self) -> None:

# invalidate cache of server notices room_ids
# if server tries to send to a cached room_id it gives an error
self.get_success(
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()
)
self.server_notices_manager.get_or_create_notice_room_for_user.invalidate_all()

# send second message
channel = self.make_request(
Expand Down
2 changes: 1 addition & 1 deletion tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
super().prepare(reactor, clock, hs)
# profile changes expect that the user is actually registered
user = UserID.from_string(self.user_id)
self.get_success(self.register_user(user.localpart, "supersecretpassword"))
self.register_user(user.localpart, "supersecretpassword")

@unittest.override_config(
{"rc_joins": {"local": {"per_second": 0.5, "burst_count": 3}}}
Expand Down
12 changes: 6 additions & 6 deletions tests/storage/test_id_generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ def test_out_of_order_finish(self) -> None:
self.assertEqual(id_gen.get_positions(), {"master": 7})
self.assertEqual(id_gen.get_current_token_for_writer("master"), 7)

ctx1 = self.get_success(id_gen.get_next())
ctx2 = self.get_success(id_gen.get_next())
ctx3 = self.get_success(id_gen.get_next())
ctx4 = self.get_success(id_gen.get_next())
ctx1 = id_gen.get_next()
ctx2 = id_gen.get_next()
ctx3 = id_gen.get_next()
ctx4 = id_gen.get_next()

s1 = self.get_success(ctx1.__aenter__())
s2 = self.get_success(ctx2.__aenter__())
Expand Down Expand Up @@ -362,8 +362,8 @@ def test_restart_during_out_of_order_persistence(self) -> None:
self.assertEqual(id_gen.get_current_token_for_writer("master"), 7)

# Persist two rows at once
ctx1 = self.get_success(id_gen.get_next())
ctx2 = self.get_success(id_gen.get_next())
ctx1 = id_gen.get_next()
ctx2 = id_gen.get_next()

s1 = self.get_success(ctx1.__aenter__())
s2 = self.get_success(ctx2.__aenter__())
Expand Down
62 changes: 20 additions & 42 deletions tests/storage/test_redaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,9 @@ def inject_redaction(self, room, event_id, user, reason):
return event

def test_redact(self):
self.get_success(
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)
)
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)

msg_event = self.get_success(self.inject_message(self.room1, self.u_alice, "t"))
msg_event = self.inject_message(self.room1, self.u_alice, "t")

# Check event has not been redacted:
event = self.get_success(self.store.get_event(msg_event.event_id))
Expand All @@ -141,9 +139,7 @@ def test_redact(self):

# Redact event
reason = "Because I said so"
self.get_success(
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)
)
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)

event = self.get_success(self.store.get_event(msg_event.event_id))

Expand All @@ -170,14 +166,10 @@ def test_redact(self):
)

def test_redact_join(self):
self.get_success(
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)
)
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)

msg_event = self.get_success(
self.inject_room_member(
self.room1, self.u_bob, Membership.JOIN, extra_content={"blue": "red"}
)
msg_event = self.inject_room_member(
self.room1, self.u_bob, Membership.JOIN, extra_content={"blue": "red"}
)

event = self.get_success(self.store.get_event(msg_event.event_id))
Expand All @@ -195,9 +187,7 @@ def test_redact_join(self):

# Redact event
reason = "Because I said so"
self.get_success(
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)
)
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)

# Check redaction

Expand Down Expand Up @@ -311,11 +301,9 @@ def internal_metadata(self):
def test_redact_censor(self):
"""Test that a redacted event gets censored in the DB after a month"""

self.get_success(
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)
)
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)

msg_event = self.get_success(self.inject_message(self.room1, self.u_alice, "t"))
msg_event = self.inject_message(self.room1, self.u_alice, "t")

# Check event has not been redacted:
event = self.get_success(self.store.get_event(msg_event.event_id))
Expand All @@ -333,9 +321,7 @@ def test_redact_censor(self):

# Redact event
reason = "Because I said so"
self.get_success(
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)
)
self.inject_redaction(self.room1, msg_event.event_id, self.u_alice, reason)

event = self.get_success(self.store.get_event(msg_event.event_id))

Expand Down Expand Up @@ -381,25 +367,19 @@ def test_redact_censor(self):
def test_redact_redaction(self):
"""Tests that we can redact a redaction and can fetch it again."""

self.get_success(
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)
)
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)

msg_event = self.get_success(self.inject_message(self.room1, self.u_alice, "t"))
msg_event = self.inject_message(self.room1, self.u_alice, "t")

first_redact_event = self.get_success(
self.inject_redaction(
self.room1, msg_event.event_id, self.u_alice, "Redacting message"
)
first_redact_event = self.inject_redaction(
self.room1, msg_event.event_id, self.u_alice, "Redacting message"
)

self.get_success(
self.inject_redaction(
self.room1,
first_redact_event.event_id,
self.u_alice,
"Redacting redaction",
)
self.inject_redaction(
self.room1,
first_redact_event.event_id,
self.u_alice,
"Redacting redaction",
)

# Now lets jump to the future where we have censored the redaction event
Expand All @@ -414,9 +394,7 @@ def test_redact_redaction(self):
def test_store_redacted_redaction(self):
"""Tests that we can store a redacted redaction."""

self.get_success(
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)
)
self.inject_room_member(self.room1, self.u_alice, Membership.JOIN)

builder = self.event_builder_factory.for_room_version(
RoomVersions.V1,
Expand Down
4 changes: 1 addition & 3 deletions tests/storage/test_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,7 @@ def prepare(self, reactor, clock, homeserver):
def _filter_messages(self, filter: JsonDict) -> List[EventBase]:
"""Make a request to /messages with a filter, returns the chunk of events."""

from_token = self.get_success(
self.hs.get_event_sources().get_current_token_for_pagination()
)
from_token = self.hs.get_event_sources().get_current_token_for_pagination()

events, next_key = self.get_success(
self.hs.get_datastores().main.paginate_room_events(
Expand Down
26 changes: 12 additions & 14 deletions tests/test_visibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,15 @@ def test_filtering(self) -> None:
#

# before we do that, we persist some other events to act as state.
self.get_success(self._inject_visibility("@admin:hs", "joined"))
self._inject_visibility("@admin:hs", "joined")
for i in range(0, 10):
self.get_success(self._inject_room_member("@resident%i:hs" % i))
self._inject_room_member("@resident%i:hs" % i)

events_to_filter = []

for i in range(0, 10):
user = "@user%i:%s" % (i, "test_server" if i == 5 else "other_server")
evt = self.get_success(
self._inject_room_member(user, extra_content={"a": "b"})
)
evt = self._inject_room_member(user, extra_content={"a": "b"})
events_to_filter.append(evt)

filtered = self.get_success(
Expand All @@ -76,10 +74,10 @@ def test_filtering(self) -> None:

def test_filter_outlier(self) -> None:
# outlier events must be returned, for the good of the collective federation
self.get_success(self._inject_room_member("@resident:remote_hs"))
self.get_success(self._inject_visibility("@resident:remote_hs", "joined"))
self._inject_room_member("@resident:remote_hs")
self._inject_visibility("@resident:remote_hs", "joined")

outlier = self.get_success(self._inject_outlier())
outlier = self._inject_outlier()
self.assertEqual(
self.get_success(
filter_events_for_server(self.storage, "remote_hs", [outlier])
Expand All @@ -88,7 +86,7 @@ def test_filter_outlier(self) -> None:
)

# it should also work when there are other events in the list
evt = self.get_success(self._inject_message("@unerased:local_hs"))
evt = self._inject_message("@unerased:local_hs")

filtered = self.get_success(
filter_events_for_server(self.storage, "remote_hs", [outlier, evt])
Expand All @@ -112,19 +110,19 @@ def test_erased_user(self) -> None:
# change in the middle of them.
events_to_filter = []

evt = self.get_success(self._inject_message("@unerased:local_hs"))
evt = self._inject_message("@unerased:local_hs")
events_to_filter.append(evt)

evt = self.get_success(self._inject_message("@erased:local_hs"))
evt = self._inject_message("@erased:local_hs")
events_to_filter.append(evt)

evt = self.get_success(self._inject_room_member("@joiner:remote_hs"))
evt = self._inject_room_member("@joiner:remote_hs")
events_to_filter.append(evt)

evt = self.get_success(self._inject_message("@unerased:local_hs"))
evt = self._inject_message("@unerased:local_hs")
events_to_filter.append(evt)

evt = self.get_success(self._inject_message("@erased:local_hs"))
evt = self._inject_message("@erased:local_hs")
events_to_filter.append(evt)

# the erasey user gets erased
Expand Down
Loading

0 comments on commit 33ebee4

Please sign in to comment.