From ea213782d8adaa21aa99f2de818172ee872cf3e9 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Wed, 4 Apr 2018 14:25:37 +1200 Subject: asyncio: remove test master.has_log Now that logs are async, using this call is almost always a mistake. Signal this by making it semi-private. The method may go away entirely down the track. --- mitmproxy/test/taddons.py | 4 ++-- test/mitmproxy/addons/test_browser.py | 3 +-- test/mitmproxy/addons/test_check_ca.py | 9 ++++++--- test/mitmproxy/proxy/protocol/test_websocket.py | 5 +++-- test/mitmproxy/proxy/test_server.py | 18 ++++++++++------- test/mitmproxy/test_addonmanager.py | 27 ++++++++++++++----------- test/mitmproxy/test_taddons.py | 4 ++-- 7 files changed, 40 insertions(+), 30 deletions(-) diff --git a/mitmproxy/test/taddons.py b/mitmproxy/test/taddons.py index 5cef80f1..6d439c4a 100644 --- a/mitmproxy/test/taddons.py +++ b/mitmproxy/test/taddons.py @@ -35,7 +35,7 @@ class RecordingMaster(mitmproxy.master.Master): for i in self.logs: print("%s: %s" % (i.level, i.msg), file=outf) - def has_log(self, txt, level=None): + def _has_log(self, txt, level=None): for i in self.logs: if level and i.level != level: continue @@ -45,7 +45,7 @@ class RecordingMaster(mitmproxy.master.Master): async def await_log(self, txt, level=None): for i in range(20): - if self.has_log(txt, level): + if self._has_log(txt, level): return True else: await asyncio.sleep(0.1) diff --git a/test/mitmproxy/addons/test_browser.py b/test/mitmproxy/addons/test_browser.py index 041bab04..b05996fe 100644 --- a/test/mitmproxy/addons/test_browser.py +++ b/test/mitmproxy/addons/test_browser.py @@ -13,9 +13,8 @@ async def test_browser(): with taddons.context() as tctx: b.start() assert po.called - b.start() - assert not tctx.master.has_log("already running") + b.start() b.browser.poll = lambda: None b.start() assert await tctx.master.await_log("already running") diff --git a/test/mitmproxy/addons/test_check_ca.py b/test/mitmproxy/addons/test_check_ca.py index cd34a9be..5e820b6d 100644 --- a/test/mitmproxy/addons/test_check_ca.py +++ b/test/mitmproxy/addons/test_check_ca.py @@ -8,12 +8,15 @@ from mitmproxy.test import taddons class TestCheckCA: @pytest.mark.parametrize('expired', [False, True]) - def test_check_ca(self, expired): + @pytest.mark.asyncio + async def test_check_ca(self, expired): msg = 'The mitmproxy certificate authority has expired!' with taddons.context() as tctx: tctx.master.server = mock.MagicMock() - tctx.master.server.config.certstore.default_ca.has_expired = mock.MagicMock(return_value=expired) + tctx.master.server.config.certstore.default_ca.has_expired = mock.MagicMock( + return_value = expired + ) a = check_ca.CheckCA() tctx.configure(a) - assert tctx.master.has_log(msg) is expired + assert await tctx.master.await_log(msg) == expired diff --git a/test/mitmproxy/proxy/protocol/test_websocket.py b/test/mitmproxy/proxy/protocol/test_websocket.py index 352a9c0d..347c0dfc 100644 --- a/test/mitmproxy/proxy/protocol/test_websocket.py +++ b/test/mitmproxy/proxy/protocol/test_websocket.py @@ -285,7 +285,8 @@ class TestPing(_WebSocketTest): wfile.flush() websockets.Frame.from_file(rfile) - def test_ping(self): + @pytest.mark.asyncio + async def test_ping(self): self.setup_connection() frame = websockets.Frame.from_file(self.client.rfile) @@ -295,7 +296,7 @@ class TestPing(_WebSocketTest): assert frame.header.opcode == websockets.OPCODE.PING assert frame.payload == b'' # We don't send payload to other end - assert self.master.has_log("Pong Received from server", "info") + assert await self.master.await_log("Pong Received from server", "info") class TestPong(_WebSocketTest): diff --git a/test/mitmproxy/proxy/test_server.py b/test/mitmproxy/proxy/test_server.py index 31033e25..bf24e28b 100644 --- a/test/mitmproxy/proxy/test_server.py +++ b/test/mitmproxy/proxy/test_server.py @@ -1,3 +1,4 @@ +import asyncio import os import socket import time @@ -234,13 +235,14 @@ class TestHTTP(tservers.HTTPProxyTest, CommonMixin): assert p.request(req) assert p.request(req) - def test_get_connection_switching(self): + @pytest.mark.asyncio + async def test_get_connection_switching(self): req = "get:'%s/p/200:b@1'" p = self.pathoc() with p.connect(): assert p.request(req % self.server.urlbase) assert p.request(req % self.server2.urlbase) - assert self.proxy.tmaster.has_log("serverdisconnect") + assert await self.proxy.tmaster.await_log("serverdisconnect") def test_blank_leading_line(self): p = self.pathoc() @@ -443,13 +445,14 @@ class TestReverse(tservers.ReverseProxyTest, CommonMixin, TcpMixin): req = self.master.state.flows[0].request assert req.host_header == "127.0.0.1" - def test_selfconnection(self): + @pytest.mark.asyncio + async def test_selfconnection(self): self.options.mode = "reverse:http://127.0.0.1:0" p = self.pathoc() with p.connect(): p.request("get:/") - assert self.master.has_log("The proxy shall not connect to itself.") + assert await self.master.await_log("The proxy shall not connect to itself.") class TestReverseSSL(tservers.ReverseProxyTest, CommonMixin, TcpMixin): @@ -549,7 +552,6 @@ class TestHttps2Http(tservers.ReverseProxyTest): p = self.pathoc(ssl=True, sni="example.com") with p.connect(): assert p.request("get:'/p/200'").status_code == 200 - assert not self.proxy.tmaster.has_log("error in handle_sni") def test_http(self): p = self.pathoc(ssl=False) @@ -814,11 +816,13 @@ class TestServerConnect(tservers.HTTPProxyTest): opts.upstream_cert = False return opts - def test_unnecessary_serverconnect(self): + @pytest.mark.asyncio + async def test_unnecessary_serverconnect(self): """A replayed/fake response with no upstream_cert should not connect to an upstream server""" self.set_addons(AFakeResponse()) assert self.pathod("200").status_code == 200 - assert not self.proxy.tmaster.has_log("serverconnect") + asyncio.sleep(0.1) + assert not self.proxy.tmaster._has_log("serverconnect") class AKillRequest: diff --git a/test/mitmproxy/test_addonmanager.py b/test/mitmproxy/test_addonmanager.py index e8e3a5f7..796ae1bd 100644 --- a/test/mitmproxy/test_addonmanager.py +++ b/test/mitmproxy/test_addonmanager.py @@ -1,4 +1,6 @@ import pytest +from unittest import mock + from mitmproxy import addons from mitmproxy import addonmanager @@ -90,22 +92,23 @@ def test_defaults(): @pytest.mark.asyncio async def test_loader(): with taddons.context() as tctx: - l = addonmanager.Loader(tctx.master) - l.add_option("custom_option", bool, False, "help") - assert "custom_option" in l.master.options + with mock.patch("mitmproxy.ctx.log.warn") as warn: + l = addonmanager.Loader(tctx.master) + l.add_option("custom_option", bool, False, "help") + assert "custom_option" in l.master.options - # calling this again with the same signature is a no-op. - l.add_option("custom_option", bool, False, "help") - assert not tctx.master.has_log("Over-riding existing option") + # calling this again with the same signature is a no-op. + l.add_option("custom_option", bool, False, "help") + assert not warn.called - # a different signature should emit a warning though. - l.add_option("custom_option", bool, True, "help") - assert await tctx.master.await_log("Over-riding existing option") + # a different signature should emit a warning though. + l.add_option("custom_option", bool, True, "help") + assert warn.called - def cmd(a: str) -> str: - return "foo" + def cmd(a: str) -> str: + return "foo" - l.add_command("test.command", cmd) + l.add_command("test.command", cmd) @pytest.mark.asyncio diff --git a/test/mitmproxy/test_taddons.py b/test/mitmproxy/test_taddons.py index 67f64463..39dd65c2 100644 --- a/test/mitmproxy/test_taddons.py +++ b/test/mitmproxy/test_taddons.py @@ -10,10 +10,10 @@ from mitmproxy import ctx @pytest.mark.asyncio async def test_recordingmaster(): with taddons.context() as tctx: - assert not tctx.master.has_log("nonexistent") + assert not tctx.master._has_log("nonexistent") assert not tctx.master.has_event("nonexistent") ctx.log.error("foo") - assert not tctx.master.has_log("foo", level="debug") + assert not tctx.master._has_log("foo", level="debug") assert await tctx.master.await_log("foo", level="error") -- cgit v1.2.3