Increase coverage for the parsec package

[kinow]

Hi,

This pull request includes changes only to test files. And there are only unit tests. The old tests for the validate module were moved into test_validate. This way they can be executed with any test runner (pytest, nose, etc), and it is also easier to gather coverage.

Running the tests in the command line:

$ cylc/lib$ PYTHONPATH=$(pwd -P) py.test parsec/tests/
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.9.1, py-1.7.0, pluggy-0.8.0
rootdir: /home/kinow/Development/python/workspace/cylc/lib, inifile:
collected 112 items                                                                                                                                                                                               

parsec/tests/test_config.py ........                                                                                                                                                                        [  7%]
parsec/tests/test_empysupport.py ..                                                                                                                                                                         [  8%]
parsec/tests/test_fileparse.py ....................                                                                                                                                                         [ 26%]
parsec/tests/test_include.py ....                                                                                                                                                                           [ 30%]
parsec/tests/test_jinja2support.py .......                                                                                                                                                                  [ 36%]
parsec/tests/test_ordered_dict.py ...........                                                                                                                                                               [ 46%]
parsec/tests/test_parsec.py ..                                                                                                                                                                              [ 48%]
parsec/tests/test_upgrade.py ...........                                                                                                                                                                    [ 58%]
parsec/tests/test_util.py .......................                                                                                                                                                           [ 78%]
parsec/tests/test_validate.py ........................                                                                                                                                                      [100%]

================================================================================================ warnings summary =================================================================================================
/home/kinow/Development/python/workspace/cylc/lib/parsec/tests/test_jinja2support.py:35: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6
  self.assertEqual("Jinja2 Error: Ops", ex.message)
/home/kinow/Development/python/workspace/cylc/lib/parsec/tests/test_jinja2support.py:41: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6
  self.assertEqual("Jinja2 CRITICAL: Ops", ex.message)

/home/kinow/Development/python/workspace/cylc/lib/parsec/tests/test_parsec.py:30: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6
  self.assertEquals(parsec_error.msg, parsec_error.message)

/home/kinow/Development/python/workspace/cylc/lib/parsec/tests/test_upgrade.py:143: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6
  self.assertEqual('ERROR: __MANY__ mismatch', cm.exception.message)

/home/kinow/Development/python/workspace/cylc/lib/parsec/tests/test_upgrade.py:128: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6
  cm.exception.message)

-- Docs: https://docs.pytest.org/en/latest/warnings.html
===================================================================================== 112 passed, 5 warnings in 0.45 seconds ======================================================================================

In my IDE, the 122 tests run in 0.211s. And it displays 74% of files covered in the parsec package, and 90% of lines.

I ignored the OrderedDictCompat.py as we will probably delete this file during the move to Python 3+.

I am hoping Travis-CI will be happy with the new tests. But I will create another branch in separate, with the coverage enabled to see how much the total coverage (which was around 55% in the other pull request) will increase for the project.

I enabled Code Compatibility in my IDE for Python 2.6, and it did detect a few errors. But I couldn't find an installation from Anaconda with Python 2.6, so alas @oliver-sanders might find some more slips of mine for that version :-(

For me these tests are enough to start hacking the files under the parsec package, and moving to Python 3 - in the near future. Planning to have a similar approach to other packages/modules... especially files like the ones containing the scheduler, suite manager, task pool, etc. This is covering just the parsing of RC and templating I think.

Cheers
Bruno

[kinow]

(note: I had a bunch of codacy-bot comments open... the moment I updated my branch they appeared as "hjoliver deleted a comment...", so it appears this happens while an analysis is happening.. the bot just goes, deletes all comments, and then once finished new comments will pop-up. When deleting, the user that appears is the repo admin, I reckon)

[kinow]

Fixed the Codacy issues that seemed easy to fix and reasonable. Others I think are OK.

Used a temporary branch merged with the coverage pull request, and it reported an overall increase of +4.84%. Going from ~54% to 59.3%, above our threshold.

Also found an issue with the coverage reports, updated the pull request for test coverage, and that might also be the solution for the weird coverage results that we had in rose.

[hjoliver]

(note: I had a bunch of codacy-bot comments open...

Ah ha! Nice detective work.

[hjoliver]

~60% coverage is heading towards "semi-reasonable" (under the circumstances)!

[matthewrmshin]

The only issue is that lib/parsec/tests/test_empysupport.py fails if EmPy is not installed. Is there any way that the test can skip if EmPy is not installed? If not, we are happy to live with it for now.

[kinow]

@matthewrmshin oh, sorry, I marked only one of the two empy tests to skip when it is not installed. Fixed by annotating the class instead.

With empy:

kinow@kinow-VirtualBox:~/Development/python/workspace/cylc$ PYTHONPATH=$(pwd -P)/lib pytest lib/parsec/tests/test_empysupport.py
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.9.1, py-1.7.0, pluggy-0.8.0
rootdir: /home/kinow/Development/python/workspace/cylc, inifile:
plugins: cov-2.6.0
collected 2 items                                                                                                                                                                                                 

lib/parsec/tests/test_empysupport.py ..

After pip uninstall empy:

kinow@kinow-VirtualBox:~/Development/python/workspace/cylc$ PYTHONPATH=$(pwd -P)/lib pytest lib/parsec/tests/test_empysupport.py
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.9.1, py-1.7.0, pluggy-0.8.0
rootdir: /home/kinow/Development/python/workspace/cylc, inifile:
plugins: cov-2.6.0
collected 2 items                                                                                                                                                                                                 

lib/parsec/tests/test_empysupport.py ss   

Instead of the dots representing the test cases, we have the s, which means marked as skipped.

👍

Sorry for that, and thanks for spotting it! Rebased branch onto master.

Bruno

[matthewrmshin]

Unit test for ListValueError broken by #2809. Quick fix looks like this, but we may want to adjust the logic of ListValueError slightly to make it a bit less clunky to use.

diff --git a/lib/parsec/tests/test_validate.py b/lib/parsec/tests/test_validate.py
index 044d94c49..dc327c9c6 100644
--- a/lib/parsec/tests/test_validate.py
+++ b/lib/parsec/tests/test_validate.py
@@ -111,20 +111,18 @@ class TestValidate(unittest.TestCase):
     def test_list_value_error(self):
         keys = ['a,', 'b', 'c']
         value = 'a sample value'
-        error = ListValueError(keys, value)
+        error = ListValueError(keys, value, 'who cares:')
         output = error.msg
-        expected = "ERROR: names containing commas must be quoted" \
-                   " (e.g. 'foo<m,n>'):\n   [a,][b]c = a sample value"
+        expected = "who cares:\n    [a,][b]c = a sample value"
         self.assertEqual(expected, output)
 
     def test_list_value_error_with_exception(self):
         keys = ['a,', 'b', 'c']
         value = 'a sample value'
         exc = Exception('test')
-        error = ListValueError(keys, value, exc)
+        error = ListValueError(keys, value, 'who cares:', exc)
         output = error.msg
-        expected = "ERROR: names containing commas must be quoted" \
-                   " (e.g. 'foo<m,n>'):\n   [a,][b]c = a sample value: test"
+        expected = "who cares:\n    [a,][b]c = a sample value: test"
         self.assertEqual(expected, output)
 
     def test_illegal_value_error(self):

[kinow]

Thanks for the fix @matthewrmshin ! It worked like a charm in my environment, so simply replicated in the branch + rebase/squash.

$ PYTHONPATH=$(pwd -P)/lib pytest lib/parsec
======================================================================================== test session starts ========================================================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.9.1, py-1.7.0, pluggy-0.8.0
rootdir: /home/kinow/Development/python/workspace/cylc, inifile:
plugins: cov-2.6.0
collected 112 items                                                                                                                                                                                 

lib/parsec/tests/test_config.py ........                                                                                                                                                      [  7%]
lib/parsec/tests/test_empysupport.py ..                                                                                                                                                       [  8%]
lib/parsec/tests/test_fileparse.py ....................                                                                                                                                       [ 26%]
lib/parsec/tests/test_include.py ....                                                                                                                                                         [ 30%]
lib/parsec/tests/test_jinja2support.py .......                                                                                                                                                [ 36%]
lib/parsec/tests/test_ordered_dict.py ...........                                                                                                                                             [ 46%]
lib/parsec/tests/test_parsec.py ..                                                                                                                                                            [ 48%]
lib/parsec/tests/test_upgrade.py ...........                                                                                                                                                  [ 58%]
lib/parsec/tests/test_util.py .......................                                                                                                                                         [ 78%]
lib/parsec/tests/test_validate.py ........................                                                                                                                                    [100%]

==================================================================================== 112 passed in 0.31 seconds =====================================================================================

[matthewrmshin]

(I'll press merge when Travis CI is done.)

[sadielbartholomew]

Sadly I have just established that at more than one of these tests does not work for Python 2.6. Compare the result of:

• PYTHONPATH=$(pwd -P)/lib python2.7 lib/parsec/tests/test_validate.py: passes
• PYTHONPATH=$(pwd -P)/lib python2.6 lib/parsec/tests/test_validate.py: multiple (10/24) assertion failures

To see the full listings, copy & run for parsec_test in *.py; do PYTHONPATH=~/cylc.git/lib python2.6 "${parsec_test}"; done from the lib/parsec/tests directory (the 'cylc.git' repo name may need to be changed to whatever you use).

However, these failures are mostly of the same instance, e.g. most commonly from TypeError: failUnlessRaises() takes at least 3 arguments (2 given) which has apparently been fixed elsewhere. I will try to amend these tests to pass for Python 2.6 now, assuming it does not take too long.

[sadielbartholomew]

most commonly from TypeError: failUnlessRaises() takes at least 3 arguments (2 given)

From the Python 2.7 docs: "Changed in version 2.7: Added the ability to use assertRaises() as a context manager." Using it as a bare assert statement would make sub-tests in question pass for v. 2.6, but I do not think this is worth the effort to amend in each instance, given we know it is a quirk of the tests & not an issue in the parsec code & especially as a grep of the current master branch shows some use of with self.assertRaises(): statements in the main Cylc codebase e.g. in lib/cylc/tests/test_c3mro.py.

However, I will still tweak any 2.6 failures not due to the above. They are, exclusively:

1. TypeError: unsupported operand type(s) for -: 'NoneType' and 'NoneType' for 3 coerce methods in test_validate.py
2. The following arising in several places:[ignore -> due to the environment variable not being picked up via the method of running the test]

   FileParseError: Jinja2Error:
     File "/usr/lib64/python2.6/UserDict.py", line 22, in __getitem__
       raise KeyError(key)
   KeyError: 'CYLC_DIR'
   

3. @unittest.skipUnless(IS_EMPY_INSTALLED, "EmPy not installed") in test_empysupport.py, where that is not available until 2.7. There does not appear to be any simple way to skip tests in 2.6.
4. test_util.py: use of the mock module which requires v. 3.3, though I seem to recall we are aware of this & okay with it?

I am now amending (1) & (2) but what, if anything, should we do about (3) and (4)? Opinions @cylc/core?

[kinow]

3. Oh, not available in 2.6? I think then if we want to fix that now, perhaps remove the annotation and try an if... maybe adding a TODO to change that later?

4. Before 3.3 it was a separate module. But I guess you cannot install that. For now I think these need to be ignored :-(

Thanks for fixing the tests on 2.6! My IDE spotted a few compatibility issues for 2.6, but without a 2.6 interpreter, it is harder to make sure it works.