Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests are needlessly nose-based and don't test anything #3

Closed
mcepl opened this issue Sep 22, 2018 · 3 comments
Closed

tests are needlessly nose-based and don't test anything #3

mcepl opened this issue Sep 22, 2018 · 3 comments

Comments

@mcepl
Copy link

mcepl commented Sep 22, 2018

First of all,

touch tests/__init__.py
mv tests/TestUrls.py tests/test_URLs.py

makes tests unittest compatible and there is no need to have additional dependency on nose (or pytest).

Second, with this patch:

--- a/tests/test_URLs.py
+++ b/tests/test_URLs.py
@@ -33,8 +33,9 @@ except ImportError:
 
 class TestUrls(unittest.TestCase):
     def setUp(self):
-        handler = urllib_gssapi.HTTPSPNEGOAuthHandler()
-        opener = urllib_request.build_opener(handler)
+        gssapi_handler = urllib_gssapi.HTTPSPNEGOAuthHandler()
+        https_handler = urllib_request.HTTPSHandler(debuglevel=2)
+        opener = urllib_request.build_opener(https_handler, gssapi_handler)
         urllib_request.install_opener(opener)
 
     def test_non_SPNEGO(self):

I can run the tests and to see what they actually do and it is not nice:

matej@stitny: urllib-gssapi-1.0.1$ python setup.py test -v
running test
running egg_info
creating lib/urllib_gssapi.egg-info
writing lib/urllib_gssapi.egg-info/PKG-INFO
writing top-level names to lib/urllib_gssapi.egg-info/top_level.txt
writing dependency_links to lib/urllib_gssapi.egg-info/dependency_links.txt
writing manifest file 'lib/urllib_gssapi.egg-info/SOURCES.txt'
file lib/urllib_gssapi.py (for module urllib_gssapi) not found
reading manifest file 'lib/urllib_gssapi.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'lib/urllib_gssapi.egg-info/SOURCES.txt'
running build_ext
test_non_SPNEGO (tests.test_URLs.TestUrls) ... send: 'GET / HTTP/1.1\r\nAccept-Encoding: identity\r\nHost: mivehind.net\r\nConnection: close\r\nUser-Agent: Python-urllib/2.7\r\n\r\n'
reply: 'HTTP/1.1 200 OK\r\n'
header: Date: Sat, 22 Sep 2018 09:34:32 GMT
header: Server: Apache/2.4.25 (Debian)
header: Last-Modified: Sat, 21 Apr 2018 05:20:51 GMT
header: ETag: "3253-56a54f7d0b2c0"
header: Accept-Ranges: bytes
header: Content-Length: 12883
header: Vary: Accept-Encoding
header: Connection: close
header: Content-Type: text/html
ok

----------------------------------------------------------------------
Ran 1 test in 1.460s

OK
matej@stitny: urllib-gssapi-1.0.1$

The whole testing conversation (and you don't use Mock and/or unittest.mock?) is nothing about SPNEGO whatsoever.

@frozencemetery
Copy link
Member

The tests are ported from urllib_kerberos, for which this is a drop-in library: https://github.com/willthames/urllib_kerberos/blob/master/tests/TestUrls.py

If you would like more to be tested, you are welcome to contribute additional tests.

@mcepl
Copy link
Author

mcepl commented Sep 24, 2018

Why does it matter that urllib_kerberos is broken as well (moreover, I believe it is widely deprecated, isn't it in favor of this package)? Any my point is not I would like to see additional tests, but that the only test which is included is a bogus one.

@simo5
Copy link

simo5 commented Sep 24, 2018

@mcepl understood #1 tracks this already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants