From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2C8EC4338F for ; Mon, 2 Aug 2021 17:26:30 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4A5EB61102 for ; Mon, 2 Aug 2021 17:26:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4A5EB61102 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:50546 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mAbht-00026C-Al for qemu-devel@archiver.kernel.org; Mon, 02 Aug 2021 13:26:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:36424) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mAbgd-0000kB-KN for qemu-devel@nongnu.org; Mon, 02 Aug 2021 13:25:12 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:29145) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mAbgX-0004hm-Dq for qemu-devel@nongnu.org; Mon, 02 Aug 2021 13:25:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1627925102; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zWV5hN1vlaaVbCnIqOVlWdQf3JCgkG2whY7mq7I9keY=; b=g4KQLse0qq1QeycwWr6m5vwty3Nem9meT4AyrXUBSkyrVK4Y9a2qdQyls/GZMvOJCy9VwE giYvfLSWiEP2wFY6XR7SLfj3MHnmpqieToN4vg7Mk9qS7V2NPYwJ1cFdPhLx+M1RRnt+Id 9vXI6J2wBGFRjJnHmOEIL0huDUfsX6M= Received: from mail-oo1-f69.google.com (mail-oo1-f69.google.com [209.85.161.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-67-CFY5Rg8XMju3OS0R4V134A-1; Mon, 02 Aug 2021 13:24:58 -0400 X-MC-Unique: CFY5Rg8XMju3OS0R4V134A-1 Received: by mail-oo1-f69.google.com with SMTP id n19-20020a4ae7530000b029024bc69d2a8aso7623865oov.16 for ; Mon, 02 Aug 2021 10:24:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zWV5hN1vlaaVbCnIqOVlWdQf3JCgkG2whY7mq7I9keY=; b=ru/QGk4zrw6a+0xdlk0AgZgMS6JxJUuzeHaWvZRAf4BDyON1ZWnZxjHhbMVvc+LcD5 NM8GNbvo/Ny0/gF1Hi9sxvXUictyei0uLslhusuU6B/ZqyvP1gdZINNpFqG9tcXd5zSK 6qrUeLj1FbBOGu7UjC5zdAEBRdmaCPxxUI1qlzLCGRJpAbJH2qMx++VVRnZ1fwyTIF+w oY/XK+cNABKcthUU8x5CVlz9obQjqMYHUyZEVch/K1A9THsJzZvi4B4poGq2zdkdHDic vNHaH/3Qetl1CQMjvxcAEswEYXgbMKJFf9N7dhCw/GXtqIrgZvPRXV5LQvDA/GjQ22Mr 4dMw== X-Gm-Message-State: AOAM531kBPzXGn5raP9OROakWVFd8J3UHjrPlGZcyXiG6Gw1SP88je7n RPZ2maGqetAoWI46zrnLdgsgPrGhSp8d8wOSf2g1ZIGodsPmK4h4UMYQVHHypqj+CJhfq/rYAHx UkeaZziXmsVvhoX3YWnJZ/SDrTyPiSgU= X-Received: by 2002:a05:6808:f94:: with SMTP id o20mr23819oiw.112.1627925097727; Mon, 02 Aug 2021 10:24:57 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzu7sIMOXC2gw0m1Z5r66Lro+LEY+kCOaOlBFtq7RSQz2PqWJEhUYHVXQ2zveR/ocKmwWYzlkjJChHdg8yOKqU= X-Received: by 2002:a05:6808:f94:: with SMTP id o20mr23801oiw.112.1627925097526; Mon, 02 Aug 2021 10:24:57 -0700 (PDT) MIME-Version: 1.0 References: <20210717003253.457418-1-jsnow@redhat.com> <20210717003253.457418-25-jsnow@redhat.com> <20210720203437.pzncze4y56phho3v@laptop.redhat> In-Reply-To: <20210720203437.pzncze4y56phho3v@laptop.redhat> From: John Snow Date: Mon, 2 Aug 2021 13:24:46 -0400 Message-ID: Subject: Re: [PATCH v2 24/24] python/aqmp: add AsyncProtocol unit tests To: Beraldo Leal Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jsnow@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="00000000000028c82805c896d905" Received-SPF: pass client-ip=216.205.24.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.701, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Willian Rampazzo , Eduardo Habkost , Markus Armbruster , Wainer dos Santos Moschetta , qemu-devel , "Niteesh G . S ." , Stefan Hajnoczi , Cleber Rosa , Eric Blake Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000028c82805c896d905 Content-Type: text/plain; charset="UTF-8" On Tue, Jul 20, 2021 at 4:34 PM Beraldo Leal wrote: > On Fri, Jul 16, 2021 at 08:32:53PM -0400, John Snow wrote: > > This tests most of protocol.py -- From a hacked up Coverage.py run, it's > > at about 86%. There's a few error cases that aren't very well tested > > yet, they're hard to induce artificially so far. I'm working on it. > > > > Signed-off-by: John Snow > > --- > > python/tests/null_proto.py | 67 ++++++ > > python/tests/protocol.py | 458 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 525 insertions(+) > > create mode 100644 python/tests/null_proto.py > > create mode 100644 python/tests/protocol.py > > > > diff --git a/python/tests/null_proto.py b/python/tests/null_proto.py > > new file mode 100644 > > index 00000000000..c697efc0001 > > --- /dev/null > > +++ b/python/tests/null_proto.py > > @@ -0,0 +1,67 @@ > > +import asyncio > > + > > +from qemu.aqmp.protocol import AsyncProtocol > > + > > + > > +class NullProtocol(AsyncProtocol[None]): > > + """ > > + NullProtocol is a test mockup of an AsyncProtocol implementation. > > + > > + It adds a fake_session instance variable that enables a code path > > + that bypasses the actual connection logic, but still allows the > > + reader/writers to start. > > + > > + Because the message type is defined as None, an asyncio.Event named > > + 'trigger_input' is created that prohibits the reader from > > + incessantly being able to yield None; this input can be poked to > > + simulate an incoming message. > > + > > + For testing symmetry with do_recv, an interface is added to "send" a > > + Null message. > > + > > + For testing purposes, a "simulate_disconnection" method is also > > + added which allows us to trigger a bottom half disconnect without > > + injecting any real errors into the reader/writer loops; in essence > > + it performs exactly half of what disconnect() normally does. > > + """ > > + def __init__(self, name=None): > > + self.fake_session = False > > + self.trigger_input: asyncio.Event > > + super().__init__(name) > > + > > + async def _establish_session(self): > > + self.trigger_input = asyncio.Event() > > + await super()._establish_session() > > + > > + async def _do_accept(self, address, ssl=None): > > + if not self.fake_session: > > + await super()._do_accept(address, ssl) > > + > > + async def _do_connect(self, address, ssl=None): > > + if not self.fake_session: > > + await super()._do_connect(address, ssl) > > + > > + async def _do_recv(self) -> None: > > + await self.trigger_input.wait() > > + self.trigger_input.clear() > > + > > + def _do_send(self, msg: None) -> None: > > + pass > > + > > + async def send_msg(self) -> None: > > + await self._outgoing.put(None) > > + > > + async def simulate_disconnect(self) -> None: > > + # Simulates a bottom half disconnect, e.g. schedules a > > + # disconnection but does not wait for it to complete. This is > > + # used to put the loop into the DISCONNECTING state without > > + # fully quiescing it back to IDLE; this is normally something > > + # you cannot coax AsyncProtocol to do on purpose, but it will be > > + # similar to what happens with an unhandled Exception in the > > + # reader/writer. > > + # > > + # Under normal circumstances, the library design requires you to > > + # await on disconnect(), which awaits the disconnect task and > > + # returns bottom half errors as a pre-condition to allowing the > > + # loop to return back to IDLE. > > + self._schedule_disconnect() > > Nitpick: Any reason for not using a docstring? I wouldn't mind if it was > a docstring instead. ;) > > Nope. I've changed it. > > diff --git a/python/tests/protocol.py b/python/tests/protocol.py > > new file mode 100644 > > index 00000000000..2374d01365e > > --- /dev/null > > +++ b/python/tests/protocol.py > > @@ -0,0 +1,458 @@ > > +import asyncio > > +from contextlib import contextmanager > > +import os > > +import socket > > +from tempfile import TemporaryDirectory > > + > > +import avocado > > + > > +from qemu.aqmp import ConnectError, Runstate > > +from qemu.aqmp.protocol import StateError > > +from qemu.aqmp.util import asyncio_run, create_task > > Nitpick: Maybe an isort? > > It actually is isorted, just using some different settings than you're used to seeing in Avocado. > > +# An Avocado bug prevents us from defining this testing class in-line > here: > > +from null_proto import NullProtocol > > Is this what you are looking for? > > https://github.com/avocado-framework/avocado/pull/4764 > > If not, can you point to the right issue, please? > > That's the one. I don't have time to update to v90 right now, so I'm going to leave it as a todo item, please forgive me! I'll update the comment, though. > > +@contextmanager > > +def jammed_socket(): > > + # This method opens up a random TCP port on localhost, then jams it. > > + socks = [] > > + > > + try: > > + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) > > + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) > > + sock.bind(('127.0.0.1', 0)) > > + sock.listen(1) > > + address = sock.getsockname() > > + > > + socks.append(sock) > > + > > + # I don't *fully* understand why, but it takes *two* un-accepted > > + # connections to start jamming the socket. > > + for _ in range(2): > > + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) > > + sock.connect(address) > > + socks.append(sock) > > + > > + yield address > > + > > + finally: > > + for sock in socks: > > + sock.close() > > + > > + > > +class Smoke(avocado.Test): > > + > > + def setUp(self): > > + self.proto = NullProtocol() > > + > > + def test__repr__(self): > > + self.assertEqual( > > + repr(self.proto), > > + "" > > + ) > > + > > + def testRunstate(self): > > + self.assertEqual( > > + self.proto.runstate, > > + Runstate.IDLE > > + ) > > + > > + def testDefaultName(self): > > + self.assertEqual( > > + self.proto.name, > > + None > > + ) > > + > > + def testLogger(self): > > + self.assertEqual( > > + self.proto.logger.name, > > + 'qemu.aqmp.protocol' > > + ) > > + > > + def testName(self): > > + self.proto = NullProtocol('Steve') > > + > > + self.assertEqual( > > + self.proto.name, > > + 'Steve' > > + ) > > + > > + self.assertEqual( > > + self.proto.logger.name, > > + 'qemu.aqmp.protocol.Steve' > > + ) > > + > > + self.assertEqual( > > + repr(self.proto), > > + "" > > + ) > > + > > + > > +class TestBase(avocado.Test): > > + > > + def setUp(self): > > + self.proto = NullProtocol(type(self).__name__) > > + self.assertEqual(self.proto.runstate, Runstate.IDLE) > > + self.runstate_watcher = None > > + > > + def tearDown(self): > > + self.assertEqual(self.proto.runstate, Runstate.IDLE) > > + > > + async def _asyncSetUp(self): > > + pass > > + > > + async def _asyncTearDown(self): > > + if self.runstate_watcher: > > + await self.runstate_watcher > > + > > + def _asyncRunner(self, test_coroutine): > > + async def coroutine(): > > + await self._asyncSetUp() > > + await test_coroutine > > + await self._asyncTearDown() > > + > > + asyncio_run(coroutine(), debug=True) > > + > > + # Definitions > > + > > + # The states we expect a "bad" connect/accept attempt to transition > through > > + BAD_CONNECTION_STATES = ( > > + Runstate.CONNECTING, > > + Runstate.DISCONNECTING, > > + Runstate.IDLE, > > + ) > > + > > + # The states we expect a "good" session to transition through > > + GOOD_CONNECTION_STATES = ( > > + Runstate.CONNECTING, > > + Runstate.RUNNING, > > + Runstate.DISCONNECTING, > > + Runstate.IDLE, > > + ) > > + > > + # Helpers > > + > > + async def _watch_runstates(self, *states): > > + # This launches a task alongside most tests below to confirm > that the > > + # sequence of runstate changes is exactly as anticipated. > > + > > + async def _watcher(): > > + for state in states: > > + new_state = await self.proto.runstate_changed() > > + self.assertEqual( > > + new_state, > > + state, > > + msg=f"Expected state '{state.name}'", > > + ) > > + > > + self.runstate_watcher = create_task(_watcher()) > > + # Kick the loop and force the task to block on the event. > > + await asyncio.sleep(0) > > + > > + > > +class State(TestBase): > > + > > + async def testSuperfluousDisconnect_(self): > > + await self._watch_runstates( > > + Runstate.DISCONNECTING, > > + Runstate.IDLE, > > + ) > > + await self.proto.disconnect() > > + > > + def testSuperfluousDisconnect(self): > > + self._asyncRunner(self.testSuperfluousDisconnect_()) > > + > > + > > +class Connect(TestBase): > > + > > + async def _bad_connection(self, family: str): > > + assert family in ('INET', 'UNIX') > > + > > + if family == 'INET': > > + await self.proto.connect(('127.0.0.1', 0)) > > + elif family == 'UNIX': > > + await self.proto.connect('/dev/null') > > + > > + async def _hanging_connection(self): > > + with jammed_socket() as addr: > > + await self.proto.connect(addr) > > + > > + async def _bad_connection_test(self, family: str): > > + await self._watch_runstates(*self.BAD_CONNECTION_STATES) > > + > > + with self.assertRaises(ConnectError) as context: > > + await self._bad_connection(family) > > + > > + self.assertIsInstance(context.exception.exc, OSError) > > + self.assertEqual( > > + context.exception.error_message, > > + "Failed to establish connection" > > + ) > > + > > + def testBadINET(self): > > + self._asyncRunner(self._bad_connection_test('INET')) > > + # self.assertIsInstance(err.exc, ConnectionRefusedError) > > + > > + def testBadUNIX(self): > > + self._asyncRunner(self._bad_connection_test('UNIX')) > > + # self.assertIsInstance(err.exc, ConnectionRefusedError) > > + > > + async def testCancellation_(self): > > + # Note that accept() cannot be cancelled outright, as it isn't > a task. > > + # However, we can wrap it in a task and cancel *that*. > > + await self._watch_runstates(*self.BAD_CONNECTION_STATES) > > + task = run_as_task(self._hanging_connection(), > allow_cancellation=True) > > + > > + state = await self.proto.runstate_changed() > > + self.assertEqual(state, Runstate.CONNECTING) > > + > > + # This is insider baseball, but the connection attempt has > > + # yielded *just* before the actual connection attempt, so kick > > + # the loop to make sure it's truly wedged. > > + await asyncio.sleep(0) > > + > > + task.cancel() > > + await task > > + > > + def testCancellation(self): > > + self._asyncRunner(self.testCancellation_()) > > + > > + async def testTimeout_(self): > > + await self._watch_runstates(*self.BAD_CONNECTION_STATES) > > + task = run_as_task(self._hanging_connection()) > > + > > + # More insider baseball: to improve the speed of this test while > > + # guaranteeing that the connection even gets a chance to start, > > + # verify that the connection hangs *first*, then await the > > + # result of the task with a nearly-zero timeout. > > + > > + state = await self.proto.runstate_changed() > > + self.assertEqual(state, Runstate.CONNECTING) > > + await asyncio.sleep(0) > > + > > + with self.assertRaises(asyncio.TimeoutError): > > + await asyncio.wait_for(task, timeout=0) > > + > > + def testTimeout(self): > > + self._asyncRunner(self.testTimeout_()) > > + > > + async def testRequire_(self): > > + await self._watch_runstates(*self.BAD_CONNECTION_STATES) > > + task = run_as_task(self._hanging_connection(), > allow_cancellation=True) > > + > > + state = await self.proto.runstate_changed() > > + self.assertEqual(state, Runstate.CONNECTING) > > + > > + with self.assertRaises(StateError) as context: > > + await self._bad_connection('UNIX') > > + > > + self.assertEqual( > > + context.exception.error_message, > > + "NullProtocol is currently connecting." > > + ) > > + self.assertEqual(context.exception.state, Runstate.CONNECTING) > > + self.assertEqual(context.exception.required, Runstate.IDLE) > > + > > + task.cancel() > > + await task > > + > > + def testRequire(self): > > + self._asyncRunner(self.testRequire_()) > > + > > + async def testImplicitRunstateInit_(self): > > + # This tests what happens if we do not wait on the > > + # runstate until AFTER we connect, i.e., connect()/accept() > > + # themselves initialize the runstate event. All of the above > > + # tests force the initialization by waiting on the runstate > > + # *first*. > > + task = run_as_task(self._hanging_connection(), > allow_cancellation=True) > > + > > + # Kick the loop to coerce the state change > > + await asyncio.sleep(0) > > + assert self.proto.runstate == Runstate.CONNECTING > > + > > + # We already missed the transition to CONNECTING > > + await self._watch_runstates(Runstate.DISCONNECTING, > Runstate.IDLE) > > + > > + task.cancel() > > + await task > > + > > + def testImplicitRunstateInit(self): > > + self._asyncRunner(self.testImplicitRunstateInit_()) > > + > > + > > +class Accept(Connect): > > + > > + async def _bad_connection(self, family: str): > > + assert family in ('INET', 'UNIX') > > + > > + if family == 'INET': > > + await self.proto.accept(('example.com', 1)) > > + elif family == 'UNIX': > > + await self.proto.accept('/dev/null') > > + > > + async def _hanging_connection(self): > > + with TemporaryDirectory(suffix='.aqmp') as tmpdir: > > + sock = os.path.join(tmpdir, type(self.proto).__name__ + > ".sock") > > + await self.proto.accept(sock) > > + > > + > > +class FakeSession(TestBase): > > + > > + def setUp(self): > > + super().setUp() > > + self.proto.fake_session = True > > + > > + async def _asyncSetUp(self): > > + await super()._asyncSetUp() > > + await self._watch_runstates(*self.GOOD_CONNECTION_STATES) > > + > > + async def _asyncTearDown(self): > > + await self.proto.disconnect() > > + await super()._asyncTearDown() > > + > > + #### > > + > > + async def testFakeConnect_(self): > > + await self.proto.connect('/not/a/real/path') > > + self.assertEqual(self.proto.runstate, Runstate.RUNNING) > > + > > + def testFakeConnect(self): > > + """Test the full state lifecycle (via connect) with a no-op > session.""" > > + self._asyncRunner(self.testFakeConnect_()) > > + > > + async def testFakeAccept_(self): > > + await self.proto.accept('/not/a/real/path') > > + self.assertEqual(self.proto.runstate, Runstate.RUNNING) > > + > > + def testFakeAccept(self): > > + """Test the full state lifecycle (via accept) with a no-op > session.""" > > + self._asyncRunner(self.testFakeAccept_()) > > + > > + async def testFakeRecv_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + logname = self.proto.logger.name > > + with self.assertLogs(logname, level='DEBUG') as context: > > + self.proto.trigger_input.set() > > + self.proto.trigger_input.clear() > > + await asyncio.sleep(0) # Kick reader. > > + > > + self.assertEqual( > > + context.output, > > + [f"DEBUG:{logname}:<-- None"], > > + ) > > + > > + def testFakeRecv(self): > > + """Test receiving a fake/null message.""" > > + self._asyncRunner(self.testFakeRecv_()) > > + > > + async def testFakeSend_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + logname = self.proto.logger.name > > + with self.assertLogs(logname, level='DEBUG') as context: > > + # Cheat: Send a Null message to nobody. > > + await self.proto.send_msg() > > + # Kick writer; awaiting on a queue.put isn't sufficient to > yield. > > + await asyncio.sleep(0) > > + > > + self.assertEqual( > > + context.output, > > + [f"DEBUG:{logname}:--> None"], > > + ) > > + > > + def testFakeSend(self): > > + """Test sending a fake/null message.""" > > + self._asyncRunner(self.testFakeSend_()) > > + > > + async def _prod_session_api( > > + self, > > + current_state: Runstate, > > + error_message: str, > > + accept: bool = True > > + ): > > + with self.assertRaises(StateError) as context: > > + if accept: > > + await self.proto.accept('/not/a/real/path') > > + else: > > + await self.proto.connect('/not/a/real/path') > > + > > + self.assertEqual(context.exception.error_message, error_message) > > + self.assertEqual(context.exception.state, current_state) > > + self.assertEqual(context.exception.required, Runstate.IDLE) > > + > > + async def testAcceptRequireRunning_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + await self._prod_session_api( > > + Runstate.RUNNING, > > + "NullProtocol is already connected and running.", > > + accept=True, > > + ) > > + > > + def testAcceptRequireRunning(self): > > + """Test that accept() cannot be called when Runstate=RUNNING""" > > + self._asyncRunner(self.testAcceptRequireRunning_()) > > + > > + async def testConnectRequireRunning_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + await self._prod_session_api( > > + Runstate.RUNNING, > > + "NullProtocol is already connected and running.", > > + accept=False, > > + ) > > + > > + def testConnectRequireRunning(self): > > + """Test that connect() cannot be called when Runstate=RUNNING""" > > + self._asyncRunner(self.testConnectRequireRunning_()) > > + > > + async def testAcceptRequireDisconnecting_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + # Cheat: force a disconnect. > > + await self.proto.simulate_disconnect() > > + > > + await self._prod_session_api( > > + Runstate.DISCONNECTING, > > + ("NullProtocol is disconnecting." > > + " Call disconnect() to return to IDLE state."), > > + accept=True, > > + ) > > + > > + def testAcceptRequireDisconnecting(self): > > + """Test that accept() cannot be called when > Runstate=DISCONNECTING""" > > + self._asyncRunner(self.testAcceptRequireDisconnecting_()) > > + > > + async def testConnectRequireDisconnecting_(self): > > + await self.proto.accept('/not/a/real/path') > > + > > + # Cheat: force a disconnect. > > + await self.proto.simulate_disconnect() > > + > > + await self._prod_session_api( > > + Runstate.DISCONNECTING, > > + ("NullProtocol is disconnecting." > > + " Call disconnect() to return to IDLE state."), > > + accept=False, > > + ) > > + > > + def testConnectRequireDisconnecting(self): > > + """Test that connect() cannot be called when > Runstate=DISCONNECTING""" > > + self._asyncRunner(self.testConnectRequireDisconnecting_()) > > -- > > 2.31.1 > > Besides that, I just would like to bring to the table that Avocado has > now a basic support for coroutines as tests that might help here. IIUC, > some of the boilerplate code (and duplicated methods) could be removed > with this: > > https://github.com/avocado-framework/avocado/pull/4788 > > In any case, I understand if the latest version is not an option here, > so: > > It's an option, it's time that is the harsh master. > Reviewed-by: Beraldo Leal > > Thanks! I updated a few bits and pieces and added the other items to a list of things to do "later". > Thanks, > -- > Beraldo > > --js --00000000000028c82805c896d905 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Jul 20, 2021 at 4:34 PM Beral= do Leal <bleal@redhat.com> wr= ote:
On Fri, Jul= 16, 2021 at 08:32:53PM -0400, John Snow wrote:
> This tests most of protocol.py -- From a hacked up Coverage.py run, it= 's
> at about 86%. There's a few error cases that aren't very well = tested
> yet, they're hard to induce artificially so far. I'm working o= n it.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 python/tests/null_proto.py |=C2=A0 67 ++++++
>=C2=A0 python/tests/protocol.py=C2=A0 =C2=A0| 458 +++++++++++++++++++++= ++++++++++++++++
>=C2=A0 2 files changed, 525 insertions(+)
>=C2=A0 create mode 100644 python/tests/null_proto.py
>=C2=A0 create mode 100644 python/tests/protocol.py
>
> diff --git a/python/tests/null_proto.py b/python/tests/null_proto.py > new file mode 100644
> index 00000000000..c697efc0001
> --- /dev/null
> +++ b/python/tests/null_proto.py
> @@ -0,0 +1,67 @@
> +import asyncio
> +
> +from qemu.aqmp.protocol import AsyncProtocol
> +
> +
> +class NullProtocol(AsyncProtocol[None]):
> +=C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 NullProtocol is a test mockup of an AsyncProtocol imple= mentation.
> +
> +=C2=A0 =C2=A0 It adds a fake_session instance variable that enables a= code path
> +=C2=A0 =C2=A0 that bypasses the actual connection logic, but still al= lows the
> +=C2=A0 =C2=A0 reader/writers to start.
> +
> +=C2=A0 =C2=A0 Because the message type is defined as None, an asyncio= .Event named
> +=C2=A0 =C2=A0 'trigger_input' is created that prohibits the r= eader from
> +=C2=A0 =C2=A0 incessantly being able to yield None; this input can be= poked to
> +=C2=A0 =C2=A0 simulate an incoming message.
> +
> +=C2=A0 =C2=A0 For testing symmetry with do_recv, an interface is adde= d to "send" a
> +=C2=A0 =C2=A0 Null message.
> +
> +=C2=A0 =C2=A0 For testing purposes, a "simulate_disconnection&qu= ot; method is also
> +=C2=A0 =C2=A0 added which allows us to trigger a bottom half disconne= ct without
> +=C2=A0 =C2=A0 injecting any real errors into the reader/writer loops;= in essence
> +=C2=A0 =C2=A0 it performs exactly half of what disconnect() normally = does.
> +=C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 def __init__(self, name=3DNone):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.fake_session =3D False
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.trigger_input: asyncio.Event
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 super().__init__(name)
> +
> +=C2=A0 =C2=A0 async def _establish_session(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.trigger_input =3D asyncio.Event() > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await super()._establish_session()
> +
> +=C2=A0 =C2=A0 async def _do_accept(self, address, ssl=3DNone):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not self.fake_session:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await super()._do_accept(ad= dress, ssl)
> +
> +=C2=A0 =C2=A0 async def _do_connect(self, address, ssl=3DNone):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not self.fake_session:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await super()._do_connect(a= ddress, ssl)
> +
> +=C2=A0 =C2=A0 async def _do_recv(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.trigger_input.wait()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.trigger_input.clear()
> +
> +=C2=A0 =C2=A0 def _do_send(self, msg: None) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pass
> +
> +=C2=A0 =C2=A0 async def send_msg(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._outgoing.put(None)
> +
> +=C2=A0 =C2=A0 async def simulate_disconnect(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Simulates a bottom half disconnect, e.g= . schedules a
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # disconnection but does not wait for it = to complete. This is
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # used to put the loop into the DISCONNEC= TING state without
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # fully quiescing it back to IDLE; this i= s normally something
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # you cannot coax AsyncProtocol to do on = purpose, but it will be
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # similar to what happens with an unhandl= ed Exception in the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # reader/writer.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 #
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Under normal circumstances, the library= design requires you to
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # await on disconnect(), which awaits the= disconnect task and
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # returns bottom half errors as a pre-con= dition to allowing the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # loop to return back to IDLE.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._schedule_disconnect()

Nitpick: Any reason for not using a docstring? I wouldn't mind if it wa= s
a docstring instead. ;)


Nope. I've changed it.
=C2=A0
> diff --git a/python/tests/protocol.py b/python/tests/protocol.py
> new file mode 100644
> index 00000000000..2374d01365e
> --- /dev/null
> +++ b/python/tests/protocol.py
> @@ -0,0 +1,458 @@
> +import asyncio
> +from contextlib import contextmanager
> +import os
> +import socket
> +from tempfile import TemporaryDirectory
> +
> +import avocado
> +
> +from qemu.aqmp import ConnectError, Runstate
> +from qemu.aqmp.protocol import StateError
> +from qemu.aqmp.util import asyncio_run, create_task

Nitpick: Maybe an isort?


It actually is isorted, just using som= e different settings than you're used to seeing in Avocado.
=C2=A0
> +# An Avocado bug prevents us from defining this testing class in-line= here:
> +from null_proto import NullProtocol

Is this what you are looking for?

https://github.com/avocado-framework/avocado/p= ull/4764

If not, can you point to the right issue, please?


That's the one. I don't have t= ime to update to v90 right now, so I'm going to leave it as a todo item= , please forgive me! I'll update the comment, though.
=C2= =A0
> +@contextmanager
> +def jammed_socket():
> +=C2=A0 =C2=A0 # This method opens up a random TCP port on localhost, = then jams it.
> +=C2=A0 =C2=A0 socks =3D []
> +
> +=C2=A0 =C2=A0 try:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 sock =3D socket.socket(socket.AF_INET, so= cket.SOCK_STREAM)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 sock.setsockopt(socket.SOL_SOCKET, socket= .SO_REUSEADDR, 1)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 sock.bind(('127.0.0.1', 0))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 sock.listen(1)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 address =3D sock.getsockname()
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 socks.append(sock)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # I don't *fully* understand why, but= it takes *two* un-accepted
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # connections to start jamming the socket= .
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for _ in range(2):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sock =3D socket.socket(sock= et.AF_INET, socket.SOCK_STREAM)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sock.connect(address)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 socks.append(sock)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 yield address
> +
> +=C2=A0 =C2=A0 finally:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for sock in socks:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sock.close()
> +
> +
> +class Smoke(avocado.Test):
> +
> +=C2=A0 =C2=A0 def setUp(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto =3D NullProtocol()
> +
> +=C2=A0 =C2=A0 def test__repr__(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 repr(self.proto),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "<NullProtocol runs= tate=3DIDLE>"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testRunstate(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.runstate,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.IDLE
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testDefaultName(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 None
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testLogger(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.logger.name<= /a>,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'qemu.aqmp.protocol'= ;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testName(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto =3D NullProtocol('Steve= 9;)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'Steve'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.logger.name<= /a>,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'qemu.aqmp.protocol.Ste= ve'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 repr(self.proto),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "<NullProtocol name= =3D'Steve' runstate=3DIDLE>"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +
> +class TestBase(avocado.Test):
> +
> +=C2=A0 =C2=A0 def setUp(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto =3D NullProtocol(type(self).__= name__)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(self.proto.runstate, Run= state.IDLE)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.runstate_watcher =3D None
> +
> +=C2=A0 =C2=A0 def tearDown(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(self.proto.runstate, Run= state.IDLE)
> +
> +=C2=A0 =C2=A0 async def _asyncSetUp(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pass
> +
> +=C2=A0 =C2=A0 async def _asyncTearDown(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if self.runstate_watcher:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.runstate_watcher=
> +
> +=C2=A0 =C2=A0 def _asyncRunner(self, test_coroutine):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 async def coroutine():
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._asyncSetUp() > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await test_coroutine
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._asyncTearDown()=
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 asyncio_run(coroutine(), debug=3DTrue) > +
> +=C2=A0 =C2=A0 # Definitions
> +
> +=C2=A0 =C2=A0 # The states we expect a "bad" connect/accept= attempt to transition through
> +=C2=A0 =C2=A0 BAD_CONNECTION_STATES =3D (
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.CONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.DISCONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.IDLE,
> +=C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 # The states we expect a "good" session to tr= ansition through
> +=C2=A0 =C2=A0 GOOD_CONNECTION_STATES =3D (
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.CONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.RUNNING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.DISCONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.IDLE,
> +=C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 # Helpers
> +
> +=C2=A0 =C2=A0 async def _watch_runstates(self, *states):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # This launches a task alongside most tes= ts below to confirm that the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # sequence of runstate changes is exactly= as anticipated.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 async def _watcher():
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for state in states:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new_state =3D= await self.proto.runstate_changed()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEq= ual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= new_state,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= state,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= msg=3Df"Expected state '{state.name}'",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.runstate_watcher =3D create_task(_wa= tcher())
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Kick the loop and force the task to blo= ck on the event.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)
> +
> +
> +class State(TestBase):
> +
> +=C2=A0 =C2=A0 async def testSuperfluousDisconnect_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.DISCONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.IDLE,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.disconnect()
> +
> +=C2=A0 =C2=A0 def testSuperfluousDisconnect(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testSuperfluousDis= connect_())
> +
> +
> +class Connect(TestBase):
> +
> +=C2=A0 =C2=A0 async def _bad_connection(self, family: str):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert family in ('INET', 'UN= IX')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if family =3D=3D 'INET':
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.connect((&= #39;127.0.0.1', 0))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 elif family =3D=3D 'UNIX':
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.connect(&#= 39;/dev/null')
> +
> +=C2=A0 =C2=A0 async def _hanging_connection(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with jammed_socket() as addr:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.connect(ad= dr)
> +
> +=C2=A0 =C2=A0 async def _bad_connection_test(self, family: str):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(*self.BAD_CON= NECTION_STATES)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertRaises(ConnectError) as c= ontext:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._bad_connection(= family)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertIsInstance(context.exception.e= xc, OSError)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context.exception.error_mes= sage,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "Failed to establish c= onnection"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testBadINET(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self._bad_connection_te= st('INET'))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # self.assertIsInstance(err.exc, Connecti= onRefusedError)
> +
> +=C2=A0 =C2=A0 def testBadUNIX(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self._bad_connection_te= st('UNIX'))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # self.assertIsInstance(err.exc, Connecti= onRefusedError)
> +
> +=C2=A0 =C2=A0 async def testCancellation_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Note that accept() cannot be cancelled = outright, as it isn't a task.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # However, we can wrap it in a task and c= ancel *that*.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(*self.BAD_CON= NECTION_STATES)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task =3D run_as_task(self._hanging_connec= tion(), allow_cancellation=3DTrue)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 state =3D await self.proto.runstate_chang= ed()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(state, Runstate.CONNECTI= NG)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # This is insider baseball, but the conne= ction attempt has
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # yielded *just* before the actual connec= tion attempt, so kick
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # the loop to make sure it's truly we= dged.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task.cancel()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await task
> +
> +=C2=A0 =C2=A0 def testCancellation(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testCancellation_(= ))
> +
> +=C2=A0 =C2=A0 async def testTimeout_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(*self.BAD_CON= NECTION_STATES)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task =3D run_as_task(self._hanging_connec= tion())
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # More insider baseball: to improve the s= peed of this test while
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # guaranteeing that the connection even g= ets a chance to start,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # verify that the connection hangs *first= *, then await the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # result of the task with a nearly-zero t= imeout.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 state =3D await self.proto.runstate_chang= ed()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(state, Runstate.CONNECTI= NG)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertRaises(asyncio.TimeoutErr= or):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.wait_for(task= , timeout=3D0)
> +
> +=C2=A0 =C2=A0 def testTimeout(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testTimeout_()) > +
> +=C2=A0 =C2=A0 async def testRequire_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(*self.BAD_CON= NECTION_STATES)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task =3D run_as_task(self._hanging_connec= tion(), allow_cancellation=3DTrue)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 state =3D await self.proto.runstate_chang= ed()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(state, Runstate.CONNECTI= NG)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertRaises(StateError) as con= text:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._bad_connection(= 'UNIX')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context.exception.error_mes= sage,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "NullProtocol is curre= ntly connecting."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(context.exception.state,= Runstate.CONNECTING)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(context.exception.requir= ed, Runstate.IDLE)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task.cancel()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await task
> +
> +=C2=A0 =C2=A0 def testRequire(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testRequire_()) > +
> +=C2=A0 =C2=A0 async def testImplicitRunstateInit_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # This tests what happens if we do not wa= it on the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # runstate until AFTER we connect, i.e., = connect()/accept()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # themselves initialize the runstate even= t. All of the above
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # tests force the initialization by waiti= ng on the runstate
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # *first*.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task =3D run_as_task(self._hanging_connec= tion(), allow_cancellation=3DTrue)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Kick the loop to coerce the state chang= e
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert self.proto.runstate =3D=3D Runstat= e.CONNECTING
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # We already missed the transition to CON= NECTING
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(Runstate.DISC= ONNECTING, Runstate.IDLE)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 task.cancel()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await task
> +
> +=C2=A0 =C2=A0 def testImplicitRunstateInit(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testImplicitRunsta= teInit_())
> +
> +
> +class Accept(Connect):
> +
> +=C2=A0 =C2=A0 async def _bad_connection(self, family: str):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert family in ('INET', 'UN= IX')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if family =3D=3D 'INET':
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept((&#= 39;exam= ple.com', 1))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 elif family =3D=3D 'UNIX':
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept(= 9;/dev/null')
> +
> +=C2=A0 =C2=A0 async def _hanging_connection(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with TemporaryDirectory(suffix=3D'.aq= mp') as tmpdir:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sock =3D os.path.join(tmpdi= r, type(self.proto).__name__ + ".sock")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept(soc= k)
> +
> +
> +class FakeSession(TestBase):
> +
> +=C2=A0 =C2=A0 def setUp(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 super().setUp()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.fake_session =3D True
> +
> +=C2=A0 =C2=A0 async def _asyncSetUp(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await super()._asyncSetUp()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._watch_runstates(*self.GOOD_CO= NNECTION_STATES)
> +
> +=C2=A0 =C2=A0 async def _asyncTearDown(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.disconnect()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await super()._asyncTearDown()
> +
> +=C2=A0 =C2=A0 ####
> +
> +=C2=A0 =C2=A0 async def testFakeConnect_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.connect('/not/a/real= /path')
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(self.proto.runstate, Run= state.RUNNING)
> +
> +=C2=A0 =C2=A0 def testFakeConnect(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test the full state lif= ecycle (via connect) with a no-op session."""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testFakeConnect_()= )
> +
> +=C2=A0 =C2=A0 async def testFakeAccept_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(self.proto.runstate, Run= state.RUNNING)
> +
> +=C2=A0 =C2=A0 def testFakeAccept(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test the full state lif= ecycle (via accept) with a no-op session."""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testFakeAccept_())=
> +
> +=C2=A0 =C2=A0 async def testFakeRecv_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 logname =3D self.proto.logger.name
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertLogs(logname, level=3D= 9;DEBUG') as context:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.trigger_input.se= t()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.proto.trigger_input.cl= ear()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)=C2= =A0 # Kick reader.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context.output,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [f"DEBUG:{logname}:<= ;-- None"],
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testFakeRecv(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test receiving a fake/n= ull message."""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testFakeRecv_()) > +
> +=C2=A0 =C2=A0 async def testFakeSend_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 logname =3D self.proto.logger.name
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertLogs(logname, level=3D= 9;DEBUG') as context:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # Cheat: Send a Null messag= e to nobody.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.send_msg()=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # Kick writer; awaiting on = a queue.put isn't sufficient to yield.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await asyncio.sleep(0)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context.output,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [f"DEBUG:{logname}:--&= gt; None"],
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testFakeSend(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test sending a fake/nul= l message."""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testFakeSend_()) > +
> +=C2=A0 =C2=A0 async def _prod_session_api(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 current_state: Runstate, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 error_message: str,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 accept: bool =3D True
> +=C2=A0 =C2=A0 ):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 with self.assertRaises(StateError) as con= text:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if accept:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.pr= oto.accept('/not/a/real/path')
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.pr= oto.connect('/not/a/real/path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(context.exception.error_= message, error_message)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(context.exception.state,= current_state)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertEqual(context.exception.requir= ed, Runstate.IDLE)
> +
> +=C2=A0 =C2=A0 async def testAcceptRequireRunning_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._prod_session_api(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.RUNNING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "NullProtocol is alrea= dy connected and running.",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 accept=3DTrue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testAcceptRequireRunning(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test that accept() cann= ot be called when Runstate=3DRUNNING"""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testAcceptRequireR= unning_())
> +
> +=C2=A0 =C2=A0 async def testConnectRequireRunning_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._prod_session_api(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.RUNNING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "NullProtocol is alrea= dy connected and running.",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 accept=3DFalse,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testConnectRequireRunning(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test that connect() can= not be called when Runstate=3DRUNNING"""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testConnectRequire= Running_())
> +
> +=C2=A0 =C2=A0 async def testAcceptRequireDisconnecting_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Cheat: force a disconnect.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.simulate_disconnect() > +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._prod_session_api(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.DISCONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ("NullProtocol is disc= onnecting."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0" Call disconnec= t() to return to IDLE state."),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 accept=3DTrue,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testAcceptRequireDisconnecting(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test that accept() cann= ot be called when Runstate=3DDISCONNECTING"""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testAcceptRequireD= isconnecting_())
> +
> +=C2=A0 =C2=A0 async def testConnectRequireDisconnecting_(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.accept('/not/a/real/= path')
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Cheat: force a disconnect.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self.proto.simulate_disconnect() > +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 await self._prod_session_api(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Runstate.DISCONNECTING,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ("NullProtocol is disc= onnecting."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0" Call disconnec= t() to return to IDLE state."),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 accept=3DFalse,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def testConnectRequireDisconnecting(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Test that connect() can= not be called when Runstate=3DDISCONNECTING"""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._asyncRunner(self.testConnectRequire= Disconnecting_())
> --
> 2.31.1

Besides that, I just would like to bring to the table that Avocado has
now a basic support for coroutines as tests that might help here. IIUC,
some of the boilerplate code (and duplicated methods) could be removed
with this:

https://github.com/avocado-framework/avocado/p= ull/4788

In any case, I understand if the latest version is not an option here,
so:


It's an option, it's time that= is the harsh master.
=C2=A0
Reviewed-by: Beraldo Leal <bleal@redhat.com>


Thanks! I updated a few bits and piece= s and added the other items to a list of things to do "later".
=C2=A0
Thanks,
--
Beraldo


--js
--00000000000028c82805c896d905--