Fix tests deprecation warnings in a backward compatible way

[atodorov]

From the original review in PR #120

@mulkieran

Please use "with self.assertRaises" everywhere. It's just easier on the eyes.

This doesn't work b/c assertRaises and assertRaisesRegex are different. I'm not quite sure what do you mean but directly changing one for the other doesn't seem to work.

Also, we always use six.PY2 and six.PY3 to check the python version for this kind of thing, not sys.version_info, so it's probably best to stick with that.

Fixed.

I think you should be able to avoid modifying the class hierarchy to deal with the versioning problem in commit 3. You might consider modifying unittest.TestCase instead, for example.

Let me see how can I do this. Is there a place where I can hook into before the test cases are actually executed? If so I can monkey-patch unittest.TestCase instead of creating another base class.

[mulkieran]

Sorry. I meant, please use a context manager with syntax like:

with self.assertRaises*(...):
    # the statement to be tested here

for all those assertRaises* calls.

[atodorov]

for all those assertRaises* calls.

Do you mean the new ones added by this change or also existing ones a well ?

[mulkieran]

I think all existing ones already use context manager.

[atodorov]

Fixed the assertRaises calls to use context manager.

The only thing left is modifying the class hierarchy to deal with the versioning problem. Btw I've updated fslabeling.py to the new syntax and LoopBackedTestCase to inherit from BlivetTestCase but for some reason I'm still getting these two errors

======================================================================
ERROR: testLabeling (formats_test.labeling_test.Ext2FSTestCase)
A sequence of tests of filesystem labeling.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/www/html/blivet/tests/formats_test/fslabeling.py", line 139, in testLabeling
    with self.assertRaisesRegex(FSError, "default label"):
AttributeError: 'Ext2FSTestCase' object has no attribute 'assertRaisesRegex'

======================================================================
ERROR: testLabeling (formats_test.labeling_test.FATFSTestCase)
A sequence of tests of filesystem labeling.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/www/html/blivet/tests/formats_test/fslabeling.py", line 139, in testLabeling
    with self.assertRaisesRegex(FSError, "default label"):
AttributeError: 'FATFSTestCase' object has no attribute 'assertRaisesRegex'

I can't figure out why this happens, all other classes which were modified to inherit from BlivetTestCase seem to work pretty well.

[mulkieran]

Instead of changing the class hierarchy, could try importing unittest and assigning to TestCase methods in tests/__init__.py.

[atodorov]

Instead of changing the class hierarchy, could try importing unittest and assigning to TestCase methods in tests/init.py.

Thanks, I didn't actually think this will work but it did. It also fixed the labeling_test errors above.

[mulkieran]

ok. this looks fine to me.

[mulkieran]

I just realized there is another consideration.

I think we should put a copy of that init.py file in every subdirectory of the test directory as well as at the top level.

The reason for this is that you might want to run your unittests more selectively, like only those from a subdirectory of tests. In that case, the top level __init__.py file may not be loaded. Or, you might want to run your tests with a more selective glob, and if the tests it finds occur only in a subdirectory that top-level __init__.py file might not be loaded.

I believe that putting that __init__.py file in every subdirectory will ensure that the correct names are available whenever the tests are called via the unittest discovery mechanism. And I believe that that code is idempotent, so duplicating it in the subdirectories should do no harm.

[atodorov]

@mulkieran - I've removed the shebang in PR #132 so we should be fine. Not sure if you'd like to document this change for anyone who wants to run the tests more selectively. (I'm not even sure if we'd like to encourage this practice)

[mulkieran]

Thanks!

Could you rebase and change the commit message to something more like this:

Make unit test assertion expressions fully Python2/3 compatible.

Use assertion methods that are defined in Python 2 and not deprecated in
Python 3 wherever possible.

Where the only choice is between a method name that is deprecated in Python
3 and one that does not exist in Python 2, use the one that does not
exist in Python 2, but translate it to the Python 2 equivalent when executing
under Python 2.

Then I should be able to push right away.

[atodorov]

should be good to go