From ffca395e48d3909c9f588767978d128c1a6529d5 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 14:44:06 +1200 Subject: Honesty in testing - Don't skip tests that fail - Don't omit console from coverage I'm working on the pathod races, and can't seem to reproduce the failures locally. Also, let's not kid ourselves about the console coverage - it should be tracked. To improve it, we should measure it. Regressions in coverage of the module should be flagged by CI. --- test/pathod/test_pathod.py | 5 ----- 1 file changed, 5 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/test_pathod.py b/test/pathod/test_pathod.py index 4d969158..5773a3c9 100644 --- a/test/pathod/test_pathod.py +++ b/test/pathod/test_pathod.py @@ -1,5 +1,4 @@ from six.moves import cStringIO as StringIO -import pytest from pathod import pathod, version from netlib import tcp @@ -129,7 +128,6 @@ class CommonTests(tutils.DaemonTests): assert self.d.last_log() # FIXME: Other binary data elements - @pytest.mark.skip(reason="race condition") def test_sizelimit(self): r = self.get("200:b@1g") assert r.status_code == 800 @@ -143,7 +141,6 @@ class CommonTests(tutils.DaemonTests): def test_info(self): assert tuple(self.d.info()["version"]) == version.IVERSION - @pytest.mark.skip(reason="race condition") def test_logs(self): assert self.d.clear_log() assert not self.d.last_log() @@ -223,7 +220,6 @@ class CommonTests(tutils.DaemonTests): ) assert r[1].payload == "test" - @pytest.mark.skip(reason="race condition") def test_websocket_frame_reflect_error(self): r, _ = self.pathoc( ["ws:/p/", "wf:-mask:knone:f'wf:b@10':i13,'a'"], @@ -233,7 +229,6 @@ class CommonTests(tutils.DaemonTests): # FIXME: Race Condition? assert "Parse error" in self.d.text_log() - @pytest.mark.skip(reason="race condition") def test_websocket_frame_disconnect_error(self): self.pathoc(["ws:/p/", "wf:b@10:d3"], ws_read_limit=0) assert self.d.last_log() -- cgit v1.2.3 From 254614e9f77e108d186ff3f7e89ec78012af65a1 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 18:10:36 +1200 Subject: Since we have locks over the logs, use direct access rather than API requests to get to them --- test/pathod/test_app.py | 2 +- test/pathod/test_pathod.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/test_app.py b/test/pathod/test_app.py index ac89c44c..7f691485 100644 --- a/test/pathod/test_app.py +++ b/test/pathod/test_app.py @@ -27,7 +27,7 @@ class TestApp(tutils.DaemonTests): def test_log(self): assert self.getpath("/log").status_code == 200 assert self.get("200:da").status_code == 200 - id = self.d.log()[0]["id"] + id = self.d.expect_log(1)[0]["id"] assert self.getpath("/log").status_code == 200 assert self.getpath("/log/%s" % id).status_code == 200 assert self.getpath("/log/9999999").status_code == 404 diff --git a/test/pathod/test_pathod.py b/test/pathod/test_pathod.py index 5773a3c9..9916984e 100644 --- a/test/pathod/test_pathod.py +++ b/test/pathod/test_pathod.py @@ -142,11 +142,10 @@ class CommonTests(tutils.DaemonTests): assert tuple(self.d.info()["version"]) == version.IVERSION def test_logs(self): - assert self.d.clear_log() - assert not self.d.last_log() + self.d.clear_log() assert self.get("202:da") - assert len(self.d.log()) == 1 - assert self.d.clear_log() + assert self.d.expect_log(1) + self.d.clear_log() assert len(self.d.log()) == 0 def test_disconnect(self): -- cgit v1.2.3 From a7522d9308a9592b0c79709f3d04cdd816ee1a57 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 18:27:51 +1200 Subject: pathod.test shouldn't use the API at all --- test/pathod/test_pathod.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/test_pathod.py b/test/pathod/test_pathod.py index 9916984e..0646d011 100644 --- a/test/pathod/test_pathod.py +++ b/test/pathod/test_pathod.py @@ -1,11 +1,15 @@ from six.moves import cStringIO as StringIO -from pathod import pathod, version +from pathod import pathod from netlib import tcp from netlib.exceptions import HttpException, TlsException import tutils +import requests.packages.urllib3 +requests.packages.urllib3.disable_warnings() + + class TestPathod(object): def test_logging(self): @@ -138,9 +142,6 @@ class CommonTests(tutils.DaemonTests): r, _ = self.pathoc([r"get:'/p/200':i0,'\r\n'"]) assert r[0].status_code == 200 - def test_info(self): - assert tuple(self.d.info()["version"]) == version.IVERSION - def test_logs(self): self.d.clear_log() assert self.get("202:da") -- cgit v1.2.3 From a04d7fd166498b54316571a4e71a58837cf11df8 Mon Sep 17 00:00:00 2001 From: Shadab Zafar Date: Thu, 2 Jun 2016 12:59:14 +0530 Subject: Py3: Return bytes from RandomGenerator and use __getitem__ rather than __getslice__ --- test/pathod/test_language_generators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'test/pathod') diff --git a/test/pathod/test_language_generators.py b/test/pathod/test_language_generators.py index 0fceae85..33ab4879 100644 --- a/test/pathod/test_language_generators.py +++ b/test/pathod/test_language_generators.py @@ -7,11 +7,12 @@ import tutils def test_randomgenerator(): g = generators.RandomGenerator("bytes", 100) assert repr(g) + assert g[0] + assert len(g[0]) == 1 assert len(g[:10]) == 10 assert len(g[1:10]) == 9 assert len(g[:1000]) == 100 assert len(g[1000:1001]) == 0 - assert g[0] def test_filegenerator(): -- cgit v1.2.3 From b510616c69135ff2e8f18da3a5fd9a19ddcdfc63 Mon Sep 17 00:00:00 2001 From: Shadab Zafar Date: Thu, 2 Jun 2016 13:00:44 +0530 Subject: Py3: Return bytes from FileGenerator and use __getitem__ instead of __getslice__ --- test/pathod/test_language_generators.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/test_language_generators.py b/test/pathod/test_language_generators.py index 33ab4879..51f55991 100644 --- a/test/pathod/test_language_generators.py +++ b/test/pathod/test_language_generators.py @@ -19,13 +19,15 @@ def test_filegenerator(): with tutils.tmpdir() as t: path = os.path.join(t, "foo") f = open(path, "wb") - f.write("x" * 10000) + f.write(b"x" * 10000) f.close() g = generators.FileGenerator(path) assert len(g) == 10000 - assert g[0] == "x" - assert g[-1] == "x" - assert g[0:5] == "xxxxx" + assert g[0] == b"x" + assert g[-1] == b"x" + assert g[0:5] == b"xxxxx" + assert len(g[1:10]) == 9 + assert len(g[10000:10001]) == 0 assert repr(g) # remove all references to FileGenerator instance to close the file # handle. -- cgit v1.2.3 From d8c52964c75afae9a7e5cbc4470159dc09f08810 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 19:32:50 +1200 Subject: Reimplement test retrievals on pathoc and ditch requests Requests uses urllib3, which has a connection pool that's not easy to disable (https://github.com/shazow/urllib3/issues/383). --- test/pathod/test_app.py | 4 ++-- test/pathod/test_pathod.py | 9 ++------- test/pathod/tutils.py | 25 +++++++++++++++++++++++-- 3 files changed, 27 insertions(+), 11 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/test_app.py b/test/pathod/test_app.py index 7f691485..fbaa773c 100644 --- a/test/pathod/test_app.py +++ b/test/pathod/test_app.py @@ -11,11 +11,11 @@ class TestApp(tutils.DaemonTests): def test_about(self): r = self.getpath("/about") - assert r.ok + assert r.status_code == 200 def test_download(self): r = self.getpath("/download") - assert r.ok + assert r.status_code == 200 def test_docs(self): assert self.getpath("/docs/pathod").status_code == 200 diff --git a/test/pathod/test_pathod.py b/test/pathod/test_pathod.py index 0646d011..ec9c169f 100644 --- a/test/pathod/test_pathod.py +++ b/test/pathod/test_pathod.py @@ -6,10 +6,6 @@ from netlib.exceptions import HttpException, TlsException import tutils -import requests.packages.urllib3 -requests.packages.urllib3.disable_warnings() - - class TestPathod(object): def test_logging(self): @@ -150,8 +146,7 @@ class CommonTests(tutils.DaemonTests): assert len(self.d.log()) == 0 def test_disconnect(self): - rsp = self.get("202:b@100k:d200") - assert len(rsp.content) < 200 + tutils.raises("unexpected eof", self.get, "202:b@100k:d200") def test_parserr(self): rsp = self.get("400:msg,b:") @@ -163,7 +158,7 @@ class CommonTests(tutils.DaemonTests): assert rsp.content.strip() == "testfile" def test_anchor(self): - rsp = self.getpath("anchor/foo") + rsp = self.getpath("/anchor/foo") assert rsp.status_code == 202 def test_invalid_first_line(self): diff --git a/test/pathod/tutils.py b/test/pathod/tutils.py index f7bb22e5..a99a2fd3 100644 --- a/test/pathod/tutils.py +++ b/test/pathod/tutils.py @@ -3,6 +3,7 @@ import re import shutil import requests from six.moves import cStringIO as StringIO +import urllib from netlib import tcp from netlib import utils @@ -66,7 +67,7 @@ class DaemonTests(object): if not (self.noweb or self.noapi): self.d.clear_log() - def getpath(self, path, params=None): + def _getpath(self, path, params=None): scheme = "https" if self.ssl else "http" resp = requests.get( "%s://localhost:%s/%s" % ( @@ -79,8 +80,28 @@ class DaemonTests(object): ) return resp + def getpath(self, path, params=None): + logfp = StringIO() + c = pathoc.Pathoc( + ("localhost", self.d.port), + ssl=self.ssl, + fp=logfp, + ) + c.connect() + if params: + path = path + "?" + urllib.urlencode(params) + resp = c.request("get:%s" % path) + return resp + def get(self, spec): - resp = requests.get(self.d.p(spec), verify=False) + logfp = StringIO() + c = pathoc.Pathoc( + ("localhost", self.d.port), + ssl=self.ssl, + fp=logfp, + ) + c.connect() + resp = c.request("get:/p/%s" % urllib.quote(spec).encode("string_escape")) return resp def pathoc( -- cgit v1.2.3 From c9dd574f48cb05ff762bccdeef2dc22c758de9ec Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 19:40:39 +1200 Subject: Silence requests warnings Once we convert the rest of the suite to pathoc, this can go. --- test/pathod/test_test.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'test/pathod') diff --git a/test/pathod/test_test.py b/test/pathod/test_test.py index cee286a4..ddb41cb3 100644 --- a/test/pathod/test_test.py +++ b/test/pathod/test_test.py @@ -4,6 +4,9 @@ from pathod import test import tutils logging.disable(logging.CRITICAL) +import requests.packages.urllib3 +requests.packages.urllib3.disable_warnings() + class TestDaemonManual: -- cgit v1.2.3 From 5fd52970fd30f00d7a7e6ebfb6ce0d4d03942ed3 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Thu, 2 Jun 2016 19:55:52 +1200 Subject: Satisfy linting gods --- test/pathod/test_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'test/pathod') diff --git a/test/pathod/test_test.py b/test/pathod/test_test.py index ddb41cb3..6399894e 100644 --- a/test/pathod/test_test.py +++ b/test/pathod/test_test.py @@ -2,10 +2,11 @@ import logging import requests from pathod import test import tutils -logging.disable(logging.CRITICAL) import requests.packages.urllib3 + requests.packages.urllib3.disable_warnings() +logging.disable(logging.CRITICAL) class TestDaemonManual: -- cgit v1.2.3 From c5076f5e019b73ec2f6efea5a4bafed90423df8f Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Fri, 3 Jun 2016 11:47:07 +1200 Subject: Implement a service connection handler counter, use it in Pathod test suite Lots of failures, but that's a good thing. --- test/pathod/tutils.py | 1 + 1 file changed, 1 insertion(+) (limited to 'test/pathod') diff --git a/test/pathod/tutils.py b/test/pathod/tutils.py index a99a2fd3..e674812b 100644 --- a/test/pathod/tutils.py +++ b/test/pathod/tutils.py @@ -64,6 +64,7 @@ class DaemonTests(object): shutil.rmtree(cls.confdir) def teardown(self): + self.d.wait_for_silence() if not (self.noweb or self.noapi): self.d.clear_log() -- cgit v1.2.3 From e60860e65d06d2b4452b7ea94902d79eed11d78c Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Fri, 3 Jun 2016 12:06:36 +1200 Subject: Make tcp.Client.connect return a context manager that closes the connection --- test/pathod/tutils.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'test/pathod') diff --git a/test/pathod/tutils.py b/test/pathod/tutils.py index e674812b..b9f38d86 100644 --- a/test/pathod/tutils.py +++ b/test/pathod/tutils.py @@ -88,11 +88,11 @@ class DaemonTests(object): ssl=self.ssl, fp=logfp, ) - c.connect() - if params: - path = path + "?" + urllib.urlencode(params) - resp = c.request("get:%s" % path) - return resp + with c.connect(): + if params: + path = path + "?" + urllib.urlencode(params) + resp = c.request("get:%s" % path) + return resp def get(self, spec): logfp = StringIO() @@ -101,9 +101,9 @@ class DaemonTests(object): ssl=self.ssl, fp=logfp, ) - c.connect() - resp = c.request("get:/p/%s" % urllib.quote(spec).encode("string_escape")) - return resp + with c.connect(): + resp = c.request("get:/p/%s" % urllib.quote(spec).encode("string_escape")) + return resp def pathoc( self, @@ -128,16 +128,16 @@ class DaemonTests(object): fp=logfp, use_http2=use_http2, ) - c.connect(connect_to) - ret = [] - for i in specs: - resp = c.request(i) - if resp: - ret.append(resp) - for frm in c.wait(): - ret.append(frm) - c.stop() - return ret, logfp.getvalue() + with c.connect(connect_to): + ret = [] + for i in specs: + resp = c.request(i) + if resp: + ret.append(resp) + for frm in c.wait(): + ret.append(frm) + c.stop() + return ret, logfp.getvalue() tmpdir = tutils.tmpdir -- cgit v1.2.3