python3 compatibility

[keszybz]

Not everything is converted. Test results:

$ py.test-2.7 --tb=no .
================ test session starts ===========================
platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /var/tmp/mirrormanager2, inifile: 
plugins: cov-2.2.1
collected 126 items 

tests/test_mdtr.py FF
tests/test_mm_lib_model.py ...................
tests/test_mmlib.py ...............................................
tests/test_mta.py F
tests/test_ui_admin.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
tests/test_ui_app.py ..FFFFFFFFFFFFF.F.F...F.
tests/test_umdl.py FF
========== 52 failed, 74 passed in 81.31 seconds ===============

py.test-3.5 --tb=no .
================== test session starts =======================
platform linux -- Python 3.5.1, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /var/tmp/mirrormanager2, inifile: 
plugins: cov-2.2.1
collected 126 items 

tests/test_mdtr.py FF
tests/test_mm_lib_model.py ...................
tests/test_mmlib.py ...............................................
tests/test_mta.py F
tests/test_ui_admin.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
tests/test_ui_app.py FFFFFFFFFFFFFFFFFFFFFFFF
tests/test_umdl.py FF
============ 60 failed, 66 passed in 62.26 seconds  =================

As you can see, tests import fine under Python 3.5, and there's only a few more failures. A bunch of tests fail because of bytes/str incompatiblity under Python 3. This is in the testing code, and I don't know what the proper solution is, so I left this alone.

mirrorlist/mirrorlist_client.wsgi runs, mirrorlist/mirrorlist_server.py fails with GeoIP import which I can't easy obtain.

Nibbles-away-at #139.

[pypingou]

Most of the changes look good, although some seem to not run on py2.

I am a little more concern about the state of the test suite, I wonder if we do not want to fix it first so that we have a clearer picture of this PR.

[keszybz]

I forgot that python2 doesn't have the builtins module, and it is provided by python2-future rpm. Using future is a bit more convenient, but in this case it's not worth the depenency, I think. I'll rework the patch to simply use normal range under python2, in this case it really doesn't matter.

I am a little more concern about the state of the test suite, I wonder if we do not want to fix it first so that we have a clearer picture of this PR.

From what I could tell, the test suite has only partial coverage, so it's not going to provide any kind of certainty anyway, unless it is significantly extended and reworked.

Most of those patches are to imports and syntax, so they're relatively safe: if they fail, they fail at import time, so if the code actually loads, it most likely works correctly too.

[keszybz]

Updated.

[Conan-Kudo]

@keszybz Since you've added commits after posting the original message with test results, have the test results improved any?

The GeoIP import issue should be resolved now, since python3-GeoIP is now available.

[keszybz]

master @ today:

$ py.test-2.7 tests --tb=no
================== test session starts =====================
platform linux2 -- Python 2.7.12, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /home/zbyszek/python/mirrormanager2, inifile: 
plugins: xdist-1.13.1
collected 126 items 

tests/test_mdtr.py FF
tests/test_mm_lib_model.py ...................
tests/test_mmlib.py ...............................................
tests/test_mta.py F
tests/test_ui_admin.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
tests/test_ui_app.py ..FFFFFFFFFFFFF.F.F...F.
tests/test_umdl.py FF

======== 52 failed, 74 passed in 50.29 seconds ==================
py.test-2.7 tests --tb=no  23.96s user 2.26s system 51% cpu 51.393 total

In my 3k branch with the following patch on top:

diff --git mirrormanager2/lib/umdl.py mirrormanager2/lib/umdl.py
index 18f62712ee..fc5e0ab4df 100644
--- mirrormanager2/lib/umdl.py
+++ mirrormanager2/lib/umdl.py
@@ -353,7 +353,7 @@ def make_repository(session, directory, relativeDName, category, target):
         if ver is None:
             if not relativeDName.endswith('/'):
                 relativeDName += '/'
-            ver = create_version_from_path(category, relativeDName)
+            ver = create_version_from_path(session, category, relativeDName)
             session.add(ver)
             session.flush()
             version_cache.append(ver)

$ py.test-2.7 tests --tb=no
================== test session starts =====================
platform linux2 -- Python 2.7.12, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /home/zbyszek/python/mirrormanager2, inifile: 
plugins: xdist-1.13.1
collected 126 items 

tests/test_mdtr.py FF
tests/test_mm_lib_model.py ...................
tests/test_mmlib.py ...............................................
tests/test_mta.py F
tests/test_ui_admin.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
tests/test_ui_app.py ..FFFFFFFFFFFFF.F.F...F.
tests/test_umdl.py FF
============= 52 failed, 74 passed in 50.08 seconds ========

$ py.test-3.5 tests --tb=no
============================= test session starts ==============================
platform linux -- Python 3.5.1, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /home/zbyszek/python/mirrormanager2, inifile: 
plugins: cov-2.2.1
collected 126 items 

tests/test_mdtr.py FF
tests/test_mm_lib_model.py ...................
tests/test_mmlib.py ...............................................
tests/test_mta.py F
tests/test_ui_admin.py FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
tests/test_ui_app.py FFFFFFFFFFFFFFFFFFFFFFFF
tests/test_umdl.py FF

==================== 60 failed, 66 passed in 46.65 seconds =====================

And again, a bunch of failures are because of bytes/str issues:

>       self.assertEqual(stdout, '')
E       AssertionError: b'' != ''

A lot of those would probably go away if the tests were changed to use universal_newlines=True or something. I can do that patch too, if you think the general direction is worth pursing and there's a chance that this'll be used at some point.

Oh, I found one more thing to fix:

>           if isinstance(admins, basestring):
E           NameError: name 'basestring' is not defined

[keszybz]

mirrorlist/mirrorlist_client.wsgi runs, mirrorlist/mirrorlist_server.py fails in handle_country_continent_redirect (new_db is empty), in both python2 and python3, it's probably because of my incomplete environement.

[Conan-Kudo]

@pypingou How does this look to you so far?

[adrianreber]

@keszybz: are you still interested in this PR? The test suite is functional again since a few months and I would like to merge this if you would rebase. If this is too old and you do not care about it anymore (understandable after almost two years) I would also cherry-pick your commits and fixup the commits with the conflicts manually.

[keszybz]

I rebased the branch, with some new commits. All the tests now pass for me on F28.

[keszybz]

Hm, rawhide build fails with "nosetests: no such command". Maybe it'd be better to switch to pytest anyway?

[keszybz]

Tests also pass in mock with python3.7 (using pytest-2 and pytest-3).

[adrianreber]

@keszybz Do you have patches to use pytest? Is that a drop-in replacement for nosetest?

[keszybz]

It's a drop-in replacement and imho the best option currently. (There's probably some niche stuff that is not entirely compatible, but pytest supports pretty much everything that the built-in unittest and also nosetests do, usually with the same syntax). Upstream nosetests development has stopped, see http://nose.readthedocs.io/en/latest/#note-to-users.

[adrianreber]

@keszybz Thanks for your work. I just changed mirrormanager to pytest and merged your pull request.