On 10/02, Tim Rühsen wrote:
Hi,

I felt free to fix / change the python test suite to succeed a 'make
distcheck'.

Please have a look at it and feel free to comment.

Darshit, I found the test suite chdir into a temp directory (testdir).
This is not the best idea, because you have to find e.g. the certs which is
only (or cleanly) possible through the 'srcdir' environment variable - which
is relative. This is why I had to use the hard-coded '..' in here:

# step one up because test suite change directory away from $srcdir (don't do
that !!!)
CERTFILE = os.path.abspath(os.path.join('..',os.getenv('srcdir','.'), 'certs',
'wget-cert.pem'))

I did not test if abs_srcdir is exported as well... (if not, we could export
it manually in Makefile.am/AM_TESTS_ENVIRONMENT).

So either we use abs_srcdir or you change the test code to not use chdir.
What do you think ?

Tim

Hi,

Sorry for the delay in responding. I had already prepared a patch to fix make distcheck on the python tests. Was waiting for the Perl tests to start working perfectly before pushing these.

Some comments about your patch though:

From bf08ac2f5c633b4adb15e1279920f6810566eecd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20R=C3=BChsen?= <[email protected]>
Date: Thu, 2 Oct 2014 15:54:16 +0200
Subject: [PATCH] fixed python test suite to succeed distcheck

---
testenv/Makefile.am                | 79 ++++++++++++++------------------------
testenv/server/http/http_server.py |  3 +-
2 files changed, 31 insertions(+), 51 deletions(-)

diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index ced19a7..a83267f 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -26,57 +26,36 @@
# as that of the covered work.


-AUTOMAKE_OPTIONS = parallel-tests
-AM_TESTS_ENVIRONMENT = MAKE_CHECK=True; export MAKE_CHECK;
-TESTS = Test-auth-basic-fail.py             \
-    Test-auth-basic.py                      \
-    Test-auth-both.py                       \
-    Test-auth-digest.py                     \
-    Test-auth-no-challenge.py               \
-    Test-auth-no-challenge-url.py           \
-    Test-auth-retcode.py                    \
-    Test-auth-with-content-disposition.py   \
-    Test-c-full.py                          \
-    Test-Content-disposition-2.py           \
-    Test-Content-disposition.py             \
-    Test-cookie-401.py                      \
-    Test-cookie-domain-mismatch.py          \
-    Test-cookie-expires.py                  \
-    Test-cookie.py                          \
-    Test-Head.py                            \
-    Test--https.py                                                     \
-    Test-O.py                               \
-    Test-Post.py                            \
+PY_TESTS = \
+    Test-auth-basic-fail.py \
+    Test-auth-basic.py \
+    Test-auth-both.py \
+    Test-auth-digest.py \
+    Test-auth-no-challenge.py \
+    Test-auth-no-challenge-url.py \
+    Test-auth-retcode.py \
+    Test-auth-with-content-disposition.py \
+    Test-c-full.py \
+    Test-Content-disposition-2.py \
+    Test-Content-disposition.py \
+    Test-cookie-401.py \
+    Test-cookie-domain-mismatch.py \
+    Test-cookie-expires.py \
+    Test-cookie.py \
+    Test-Head.py \
+    Test--https.py \
+    Test-O.py \
+    Test-Post.py \
    Test--spider-r.py

-XFAIL_TESTS = Test-auth-both.py
+PY_XFAIL_TESTS = Test-auth-both.py

-LOG_COMPILER = python3
+EXTRA_DIST = $(PY_TESTS) $(PY_XFAIL_TESTS) certs conf exc misc server test

-EXTRA_DIST = ColourTerm.py          \
-    FTPServer.py                \
-    HTTPServer.py               \
-    README                  \
-    Test--spider-r.py           \
-    Test--https.py                             \
-    Test-Content-disposition-2.py       \
-    Test-Content-disposition.py     \
-    Test-Head.py                \
-    Test-O.py               \
-    Test-Parallel-Proto.py          \
-    Test-Post.py                \
-    Test-Proto.py               \
-    Test-auth-basic-fail.py         \
-    Test-auth-basic.py          \
-    Test-auth-both.py           \
-    Test-auth-digest.py         \
-    Test-auth-no-challenge-url.py       \
-    Test-auth-no-challenge.py       \
-    Test-auth-retcode.py            \
-    Test-auth-with-content-disposition.py   \
-    Test-c-full.py              \
-    Test-cookie-401.py          \
-    Test-cookie-domain-mismatch.py      \
-    Test-cookie-expires.py          \
-    Test-cookie.py              \
-    WgetTest.py
+TESTS = $(PY_TESTS)
+XFAIL_TESTS = $(PY_XFAIL_TESTS)
The automake environment, if I understood it correctly requires *all* tests to be listed in the TESTS variable. The XFAIL_TESTS var is supposed to list the expected failures. Hence, $(PY_XFAIL_TESTS) must be listed under TESTS too.

Also, I would prefer that XFAIL_TESTS explicitly lists the tests which are failing since this variable would be frequently changed as the status of tests change.
+
+TEST_EXTENSIONS = .py
+AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; export 
SYSTEM_WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK; export 
PYTHONPATH=$$PYTHONPATH:$(srcdir);
The SYSTEM_WGETRC environment variable is not required for the Python tests because all the tests are invoked with the --no-config command line option.
+PY_LOG_COMPILER = python3
+#AM_PY_LOG_FLAGS = diff --git a/testenv/server/http/http_server.py b/testenv/server/http/http_server.py
index 12e0434..b4237ef 100644
--- a/testenv/server/http/http_server.py
+++ b/testenv/server/http/http_server.py
@@ -39,7 +39,8 @@ class HTTPSServer (StoppableHTTPServer):
    def __init__ (self, address, handler):
        BaseServer.__init__ (self, address, handler)
        print (os.getcwd())
-        CERTFILE = os.path.abspath(os.path.join('..', 'certs', 
'wget-cert.pem'))
+        # step one up because test suite change directory away from $srcdir 
(don't do that !!!)
+        CERTFILE = os.path.abspath(os.path.join('..',os.getenv('srcdir','.'), 
'certs', 'wget-cert.pem'))
        print (CERTFILE)
        fop = open (CERTFILE)
        print (fop.readline())
--
2.1.1

--- end quoted text ---

I've attached my version of the fixes and another patch that lends some code cleanup to the python tests.

Do check it out. I'll push it soon.

--
Thanking You,
Darshit Shah
From 5f1cb001143d89f75dd6cf3e84d5ab869d05010d Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Wed, 8 Oct 2014 16:47:21 +0530
Subject: [PATCH 1/2] Fix make distcheck for Python tests

---
 testenv/ChangeLog                  |  6 ++++++
 testenv/Makefile.am                | 32 +++-----------------------------
 testenv/server/http/http_server.py |  5 +----
 3 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/testenv/ChangeLog b/testenv/ChangeLog
index f477314..ed58a69 100644
--- a/testenv/ChangeLog
+++ b/testenv/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-08  Darshit Shah  <[email protected]>
+
+	* Makefile.am: Fix EXTRA_DIST variable for make distcheck
+	* server/http/http_server.py (HTTPServer.__init__):  Fix how CERTFILE is
+	found when running make dist / make distcheck.
+
 2014-09-30  Tim Ruehsen  <[email protected]>
 
 	* test/base_test.py: Add --track-origins=yes to valgrind testing
diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index ced19a7..7cf69b5 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -27,7 +27,7 @@
 
 
 AUTOMAKE_OPTIONS = parallel-tests
-AM_TESTS_ENVIRONMENT = MAKE_CHECK=True; export MAKE_CHECK;
+AM_TESTS_ENVIRONMENT = MAKE_CHECK=True; export MAKE_CHECK; srcdir=$(srcdir); export srcdir;
 TESTS = Test-auth-basic-fail.py             \
     Test-auth-basic.py                      \
     Test-auth-both.py                       \
@@ -44,7 +44,7 @@ TESTS = Test-auth-basic-fail.py             \
     Test-cookie-expires.py                  \
     Test-cookie.py                          \
     Test-Head.py                            \
-    Test--https.py							\
+    Test--https.py                          \
     Test-O.py                               \
     Test-Post.py                            \
     Test--spider-r.py
@@ -53,30 +53,4 @@ XFAIL_TESTS = Test-auth-both.py
 
 LOG_COMPILER = python3
 
-EXTRA_DIST = ColourTerm.py          \
-    FTPServer.py                \
-    HTTPServer.py               \
-    README                  \
-    Test--spider-r.py           \
-    Test--https.py				\
-    Test-Content-disposition-2.py       \
-    Test-Content-disposition.py     \
-    Test-Head.py                \
-    Test-O.py               \
-    Test-Parallel-Proto.py          \
-    Test-Post.py                \
-    Test-Proto.py               \
-    Test-auth-basic-fail.py         \
-    Test-auth-basic.py          \
-    Test-auth-both.py           \
-    Test-auth-digest.py         \
-    Test-auth-no-challenge-url.py       \
-    Test-auth-no-challenge.py       \
-    Test-auth-retcode.py            \
-    Test-auth-with-content-disposition.py   \
-    Test-c-full.py              \
-    Test-cookie-401.py          \
-    Test-cookie-domain-mismatch.py      \
-    Test-cookie-expires.py          \
-    Test-cookie.py              \
-    WgetTest.py
+EXTRA_DIST = certs conf exc misc server test README $(TESTS)
diff --git a/testenv/server/http/http_server.py b/testenv/server/http/http_server.py
index 12e0434..569cae2 100644
--- a/testenv/server/http/http_server.py
+++ b/testenv/server/http/http_server.py
@@ -11,7 +11,6 @@ import re
 import ssl
 import os
 
-
 class StoppableHTTPServer (HTTPServer):
     """ This class extends the HTTPServer class from default http.server library
     in Python 3. The StoppableHTTPServer class is capable of starting an HTTP
@@ -38,9 +37,7 @@ class HTTPSServer (StoppableHTTPServer):
 
     def __init__ (self, address, handler):
         BaseServer.__init__ (self, address, handler)
-        print (os.getcwd())
-        CERTFILE = os.path.abspath(os.path.join('..', 'certs', 'wget-cert.pem'))
-        print (CERTFILE)
+        CERTFILE = os.path.abspath(os.path.join('..', os.getenv('srcdir', '.'), 'certs', 'wget-cert.pem'))
         fop = open (CERTFILE)
         print (fop.readline())
         self.socket = ssl.wrap_socket (
-- 
2.1.2

From 68c0fbf13ca1705504f1b59ca7420bedd2fe2d5e Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Wed, 8 Oct 2014 16:57:22 +0530
Subject: [PATCH 2/2] Minor optimizations of Python tests

---
 testenv/ChangeLog                  | 15 +++++++++++++
 testenv/Makefile.am                |  1 +
 testenv/conf/__init__.py           |  5 ++---
 testenv/misc/colour_terminal.py    | 10 ++++-----
 testenv/server/http/http_server.py | 44 ++++++--------------------------------
 testenv/test/base_test.py          |  3 +--
 6 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/testenv/ChangeLog b/testenv/ChangeLog
index ed58a69..18087b6 100644
--- a/testenv/ChangeLog
+++ b/testenv/ChangeLog
@@ -1,3 +1,18 @@
+2014-10-01  Darshit Shah  <[email protected]>
+
+	* Makefile.am: Run the tests in Python's Optimizedmode
+	* conf/__init__.py (gen_hook): Use try..except instead of if..else
+	* misc/color_terminal.py: System and check will not change while a test is
+	run. Do not test for them on every invokation of printer()
+	* server/http/http_server.py: The ssl and re modules are required by
+	specific functions. Load them lazily
+	(HTTPSServer.__init__): Lazy load ssl module here
+	(_handler.parse_range_header): Lazy load re module here
+	(_Handler.get_rule_list): get() can return a default value. Use it
+	(_Handler.guess_type): Same
+	(_Handler.is_authorized): Unused function artefact. Remove
+	(_Handler.reject_headers): Unused function artefact. Remove
+
 2014-10-08  Darshit Shah  <[email protected]>
 
 	* Makefile.am: Fix EXTRA_DIST variable for make distcheck
diff --git a/testenv/Makefile.am b/testenv/Makefile.am
index 7cf69b5..09320b0 100644
--- a/testenv/Makefile.am
+++ b/testenv/Makefile.am
@@ -52,5 +52,6 @@ TESTS = Test-auth-basic-fail.py             \
 XFAIL_TESTS = Test-auth-both.py
 
 LOG_COMPILER = python3
+AM_LOG_FLAGS = -O
 
 EXTRA_DIST = certs conf exc misc server test README $(TESTS)
diff --git a/testenv/conf/__init__.py b/testenv/conf/__init__.py
index 603bd62..4b5ddc4 100644
--- a/testenv/conf/__init__.py
+++ b/testenv/conf/__init__.py
@@ -3,7 +3,6 @@ import os
 # this file implements the mechanism of conf class auto-registration,
 # don't modify this file if you have no idea what you're doing
 
-
 def gen_hook():
     hook_table = {}
 
@@ -24,9 +23,9 @@ def gen_hook():
             return cls
 
     def find_hook(name):
-        if name in hook_table:
+        try:
             return hook_table[name]
-        else:
+        except:
             raise AttributeError
 
     return Wrapper, find_hook
diff --git a/testenv/misc/colour_terminal.py b/testenv/misc/colour_terminal.py
index cfbae94..b0849c1 100644
--- a/testenv/misc/colour_terminal.py
+++ b/testenv/misc/colour_terminal.py
@@ -25,14 +25,12 @@ T_COLORS = {
     'ENDC'   : '\033[0m'
 }
 
+system = True if platform.system() == 'Linux' else False
+check = False if getenv("MAKE_CHECK") == 'True' else True
 
 def printer (color, string):
-    if platform.system () == 'Linux':
-        if getenv ("MAKE_CHECK", "False") == "True":
-            print (string)
-        else:
-            print (T_COLORS.get (color) + string + T_COLORS.get ('ENDC'))
-
+    if system and check:
+        print (T_COLORS.get (color) + string + T_COLORS.get ('ENDC'))
     else:
         print (string)
 
diff --git a/testenv/server/http/http_server.py b/testenv/server/http/http_server.py
index 569cae2..22141f6 100644
--- a/testenv/server/http/http_server.py
+++ b/testenv/server/http/http_server.py
@@ -7,10 +7,9 @@ from random import random
 from hashlib import md5
 import threading
 import socket
-import re
-import ssl
 import os
 
+
 class StoppableHTTPServer (HTTPServer):
     """ This class extends the HTTPServer class from default http.server library
     in Python 3. The StoppableHTTPServer class is capable of starting an HTTP
@@ -36,6 +35,7 @@ class HTTPSServer (StoppableHTTPServer):
     additional support for secure connections through SSL. """
 
     def __init__ (self, address, handler):
+        import ssl
         BaseServer.__init__ (self, address, handler)
         CERTFILE = os.path.abspath(os.path.join('..', os.getenv('srcdir', '.'), 'certs', 'wget-cert.pem'))
         fop = open (CERTFILE)
@@ -59,8 +59,7 @@ class _Handler (BaseHTTPRequestHandler):
     requests. """
 
     def get_rule_list (self, name):
-        r_list = self.rules.get (name) if name in self.rules else None
-        return r_list
+        return self.rules.get(name)
 
     # The defailt protocol version of the server we run is HTTP/1.1 not
     # HTTP/1.0 which is the default with the http.server module.
@@ -133,6 +132,7 @@ class _Handler (BaseHTTPRequestHandler):
     """ Helper functions for the Handlers. """
 
     def parse_range_header (self, header_line, length):
+        import re
         if header_line is None:
             return None
         if not header_line.startswith ("bytes="):
@@ -314,23 +314,6 @@ class _Handler (BaseHTTPRequestHandler):
         if is_auth is False:
             raise ServerError ("Unable to Authenticate")
 
-    def is_authorized (self):
-        is_auth = True
-        auth_rule = self.get_rule_list ('Authentication')
-        if auth_rule:
-            auth_header = self.headers.get ("Authorization")
-            req_auth = auth_rule.auth_type
-            if req_auth == "Both" or req_auth == "Both_inline":
-                auth_type = auth_header.split(' ')[0] if auth_header else req_auth
-            else:
-                auth_type = req_auth
-            assert hasattr (self, "authorize_" + auth_type)
-            is_auth = getattr (self, "authorize_" + auth_type) (auth_header, auth_rule)
-            if is_auth is False:
-                self.send_response (401)
-                self.send_challenge (auth_type)
-                self.finish_headers ()
-        return is_auth
 
     def ExpectHeader (self, header_obj):
         exp_headers = header_obj.headers
@@ -341,6 +324,7 @@ class _Handler (BaseHTTPRequestHandler):
                 self.finish_headers ()
                 raise ServerError ("Header " + header_line + " not found")
 
+
     def RejectHeader (self, header_obj):
         rej_headers = header_obj.headers
         for header_line in rej_headers:
@@ -350,18 +334,6 @@ class _Handler (BaseHTTPRequestHandler):
                 self.finish_headers ()
                 raise ServerError ("Header " + header_line + ' received')
 
-    def reject_headers (self):
-        rej_headers = self.get_rule_list ("RejectHeader")
-        if rej_headers:
-            rej_headers = rej_headers.headers
-            for header_line in rej_headers:
-                header_re = self.headers.get (header_line)
-                if header_re is not None and header_re == rej_headers[header_line]:
-                    self.send_error (400, 'Blacklisted Header was Sent')
-                    self.end_headers ()
-                    return False
-        return True
-
     def __log_request (self, method):
         req = method + " " + self.path
         self.server.request_headers.append (req)
@@ -437,11 +409,7 @@ class _Handler (BaseHTTPRequestHandler):
             ".css"   :   "text/css",
             ".html"  :   "text/html"
         }
-        if ext in extension_map:
-            return extension_map[ext]
-        else:
-            return "text/plain"
-
+        return extension_map.get(ext, "text/plain")
 
 class HTTPd (threading.Thread):
     server_class = StoppableHTTPServer
diff --git a/testenv/test/base_test.py b/testenv/test/base_test.py
index 14143c2..665b3c5 100644
--- a/testenv/test/base_test.py
+++ b/testenv/test/base_test.py
@@ -116,9 +116,8 @@ class BaseTest:
             # 1 a
             # 5 e
             # 3 c
-            protocol = protocol.lower()
             for url in urls:
-                cmd_line += '%s://%s/%s ' % (protocol, domain, url)
+                cmd_line += '%s://%s/%s ' % (protocol.lower(), domain, url)
 
         print(cmd_line)
 
-- 
2.1.2

Attachment: pgpvrUo4oy5bV.pgp
Description: PGP signature

Reply via email to