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

async tests aren't getting run in trial-olddeps #15430

Closed
squahtx opened this issue Apr 13, 2023 · 2 comments
Closed

async tests aren't getting run in trial-olddeps #15430

squahtx opened this issue Apr 13, 2023 · 2 comments
Labels
A-CI Issues related to CI on the Synapse repository A-Testing Issues related to testing in complement, synapse, etc O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Apr 13, 2023

See https://github.com/matrix-org/synapse/actions/runs/4677776999/jobs/8285741624#step:11:17 for an example.

Exception ignored in: <coroutine object TestSSOHandler.test_set_avatar at 0x7f72088d9c20>
  Traceback (most recent call last):
    File "/opt/hostedtoolcache/Python/3.7.16/x64/lib/python3.7/warnings.py", line 5[18](https://github.com/matrix-org/synapse/actions/runs/4677776999/jobs/8285741624#step:11:19), in _warn_unawaited_coroutine
      warn(msg, category=RuntimeWarning, stacklevel=2, source=coro)
  RuntimeWarning: coroutine 'TestSSOHandler.test_set_avatar' was never awaited

The version of trial in the olddeps job likely doesn't support async test cases.

@squahtx squahtx added A-Testing Issues related to testing in complement, synapse, etc A-CI Issues related to CI on the Synapse repository T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Apr 13, 2023
@clokep
Copy link
Member

clokep commented Apr 13, 2023

The version of trial in the olddeps job likely doesn't support async test cases.

I think only Twisted-latest allowed this, we shouldn't be using async def for test-cases.

@reivilibre
Copy link
Contributor

As far as I can tell, this is good now, thanks to @dklimpel in #15433. Thanks!!

$ rg 'async def test_' -C 25
tests/rest/client/test_third_party_rules.py
403-                "config": {},
404-            }
405-        }
406-    )
407-    def test_legacy_on_create_room(self) -> None:
408-        """Tests that the wrapper for legacy on_create_room callbacks works
409-        correctly.
410-        """
411-        self.helper.create_room_as(self.user_id, tok=self.tok, expect_code=403)
412-
413-    def test_sent_event_end_up_in_room_state(self) -> None:
414-        """Tests that a state event sent by a module while processing another state event
415-        doesn't get dropped from the state of the room. This is to guard against a bug
416-        where Synapse has been observed doing so, see https://github.com/matrix-org/synapse/issues/10830
417-        """
418-        event_type = "org.matrix.test_state"
419-
420-        # This content will be updated later on, and since we actually use a reference on
421-        # the dict it does the right thing. It's a bit hacky but a handy way of making
422-        # sure the state actually gets updated.
423-        event_content = {"i": -1}
424-
425-        api = self.hs.get_module_api()
426-
427-        # Define a callback that sends a custom event on power levels update.
428:        async def test_fn(
429-            event: EventBase, state_events: StateMap[EventBase]
430-        ) -> Tuple[bool, Optional[JsonDict]]:
431-            if event.is_state() and event.type == EventTypes.PowerLevels:
432-                await api.create_and_send_event_into_room(
433-                    {
434-                        "room_id": event.room_id,
435-                        "sender": event.sender,
436-                        "type": event_type,
437-                        "content": event_content,
438-                        "state_key": "",
439-                    }
440-                )
441-            return True, None
442-
443-        self.hs.get_third_party_event_rules()._check_event_allowed_callbacks = [test_fn]
444-
445-        # Sometimes the bug might not happen the first time the event type is added
446-        # to the state but might happen when an event updates the state of the room for
447-        # that type, so we test updating the state several times.
448-        for i in range(5):
449-            # Update the content of the custom state event to be sent by the callback.
450-            event_content["i"] = i
451-
452-            # Update the room's power levels with a different value each time so Synapse
453-            # doesn't consider an update redundant.

tests/storage/test_id_generators.py
46-            );
47-            """
48-        )
49-        txn.execute("INSERT INTO foobar VALUES (123, 'hello world');")
50-
51-    def _create_id_generator(self) -> StreamIdGenerator:
52-        def _create(conn: LoggingDatabaseConnection) -> StreamIdGenerator:
53-            return StreamIdGenerator(
54-                db_conn=conn,
55-                notifier=self.hs.get_replication_notifier(),
56-                table="foobar",
57-                column="stream_id",
58-            )
59-
60-        return self.get_success_or_raise(self.db_pool.runWithConnection(_create))
61-
62-    def test_initial_value(self) -> None:
63-        """Check that we read the current token from the DB."""
64-        id_gen = self._create_id_generator()
65-        self.assertEqual(id_gen.get_current_token(), 123)
66-
67-    def test_single_gen_next(self) -> None:
68-        """Check that we correctly increment the current token from the DB."""
69-        id_gen = self._create_id_generator()
70-
71:        async def test_gen_next() -> None:
72-            async with id_gen.get_next() as next_id:
73-                # We haven't persisted `next_id` yet; current token is still 123
74-                self.assertEqual(id_gen.get_current_token(), 123)
75-                # But we did learn what the next value is
76-                self.assertEqual(next_id, 124)
77-
78-            # Once the context manager closes we assume that the `next_id` has been
79-            # written to the DB.
80-            self.assertEqual(id_gen.get_current_token(), 124)
81-
82-        self.get_success(test_gen_next())
83-
84-    def test_multiple_gen_nexts(self) -> None:
85-        """Check that we handle overlapping calls to gen_next sensibly."""
86-        id_gen = self._create_id_generator()
87-
88:        async def test_gen_next() -> None:
89-            ctx1 = id_gen.get_next()
90-            ctx2 = id_gen.get_next()
91-            ctx3 = id_gen.get_next()
92-
93-            # Request three new stream IDs.
94-            self.assertEqual(await ctx1.__aenter__(), 124)
95-            self.assertEqual(await ctx2.__aenter__(), 125)
96-            self.assertEqual(await ctx3.__aenter__(), 126)
97-
98-            # None are persisted: current token unchanged.
99-            self.assertEqual(id_gen.get_current_token(), 123)
100-
101-            # Persist each in turn.
102-            await ctx1.__aexit__(None, None, None)
103-            self.assertEqual(id_gen.get_current_token(), 124)
104-            await ctx2.__aexit__(None, None, None)
105-            self.assertEqual(id_gen.get_current_token(), 125)
106-            await ctx3.__aexit__(None, None, None)
107-            self.assertEqual(id_gen.get_current_token(), 126)
108-
109-        self.get_success(test_gen_next())
110-
111-    def test_multiple_gen_nexts_closed_in_different_order(self) -> None:
112-        """Check that we handle overlapping calls to gen_next, even when their IDs
113-        created and persisted in different orders."""
114-        id_gen = self._create_id_generator()
115-
116:        async def test_gen_next() -> None:
117-            ctx1 = id_gen.get_next()
118-            ctx2 = id_gen.get_next()
119-            ctx3 = id_gen.get_next()
120-
121-            # Request three new stream IDs.
122-            self.assertEqual(await ctx1.__aenter__(), 124)
123-            self.assertEqual(await ctx2.__aenter__(), 125)
124-            self.assertEqual(await ctx3.__aenter__(), 126)
125-
126-            # None are persisted: current token unchanged.
127-            self.assertEqual(id_gen.get_current_token(), 123)
128-
129-            # Persist them in a different order, starting with 126 from ctx3.
130-            await ctx3.__aexit__(None, None, None)
131-            # We haven't persisted 124 from ctx1 yet---current token is still 123.
132-            self.assertEqual(id_gen.get_current_token(), 123)
133-
134-            # Now persist 124 from ctx1.
135-            await ctx1.__aexit__(None, None, None)
136-            # Current token is then 124, waiting for 125 to be persisted.
137-            self.assertEqual(id_gen.get_current_token(), 124)
138-
139-            # Finally persist 125 from ctx2.
140-            await ctx2.__aexit__(None, None, None)
141-            # Current token is then 126 (skipping over 125).
142-            self.assertEqual(id_gen.get_current_token(), 126)
143-
144-        self.get_success(test_gen_next())
145-
146-    def test_gen_next_while_still_waiting_for_persistence(self) -> None:
147-        """Check that we handle overlapping calls to gen_next."""
148-        id_gen = self._create_id_generator()
149-
150:        async def test_gen_next() -> None:
151-            ctx1 = id_gen.get_next()
152-            ctx2 = id_gen.get_next()
153-            ctx3 = id_gen.get_next()
154-
155-            # Request two new stream IDs.
156-            self.assertEqual(await ctx1.__aenter__(), 124)
157-            self.assertEqual(await ctx2.__aenter__(), 125)
158-
159-            # Persist ctx2 first.
160-            await ctx2.__aexit__(None, None, None)
161-            # Still waiting on ctx1's ID to be persisted.
162-            self.assertEqual(id_gen.get_current_token(), 123)
163-
164-            # Now request a third stream ID. It should be 126 (the smallest ID that
165-            # we've not yet handed out.)
166-            self.assertEqual(await ctx3.__aenter__(), 126)
167-
168-        self.get_success(test_gen_next())
169-
170-
171-class MultiWriterIdGeneratorTestCase(HomeserverTestCase):
172-    if not USE_POSTGRES_FOR_TESTS:
173-        skip = "Requires Postgres"
174-
175-    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:

All the remaining async def test_ I can see look like red herrings — they are used as part of tests but aren't themselves tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CI Issues related to CI on the Synapse repository A-Testing Issues related to testing in complement, synapse, etc O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

3 participants