Simplify more tests with parameterization

Repetitive parser tests were repetitive. Only small amounts of cleverness makes
them non-repetitive.

salve/action/base.py

@@ -74,7 +74,7 @@ def get_verification_code(self, filesys):
         ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)
         return self.verification_codes.OK
 
-    def canexec_non_ok_code(self, code):
+    def canexec_non_ok_code(self, code):  # pragma: no cover
         """
         Do verification on a non-OK verification code.
         Essentially acts as a generic handler for all of the common conditions

salve/action/copy/base.py

@@ -44,8 +44,6 @@ def canexec_non_ok_code(self, code):
                 '{0}: {1}: Non-Readable source "{2}"'
                 .format(self.file_context, self.prettyname, self.src))
             return False
-        else:
-            assert False
 
     def __str__(self):
         return ("{0}(src={1},dst={2},context={3!r})".format(

tests/unit/action/modify_test.py

@@ -41,6 +41,14 @@ def arglist_from_mock(mock_func):
     return [args[0] for args in mock_func.call_args_list]
 
 
+stringification_params = [
+    ('Unit: FileChownAction String Conversion', modify.FileChownAction),
+    ('Unit: FileChmodAction String Conversion', modify.FileChmodAction),
+    ('Unit: DirChownAction String Conversion', modify.DirChownAction),
+    ('Unit: DirChmodAction String Conversion', modify.DirChmodAction),
+]
+
+
 class TestWithScratchdir(scratch.ScratchContainer):
     def __init__(self):
         scratch.ScratchContainer.__init__(self)
@@ -415,33 +423,25 @@ def dirchmod_execute_nonrecursive_owner(self, mock_verify, mock_chmod):
         chmod_args = arglist_from_mock(mock_chmod)
         eq_(chmod_args, [('a', int('755', 8))])
 
-    @parameterized.expand(
-        [('Unit: FileChownAction String Conversion', modify.FileChownAction,
-          'FileChownAction'),
-         ('Unit: FileChmodAction String Conversion', modify.FileChmodAction,
-          'FileChmodAction'),
-         ('Unit: DirChownAction String Conversion', modify.DirChownAction,
-          'DirChownAction'),
-         ('Unit: DirChmodAction String Conversion', modify.DirChmodAction,
-          'DirChmodAction'),
-         ],
-        testcase_func_doc=first_param_docfunc)
+    @parameterized.expand(stringification_params,
+                          testcase_func_doc=first_param_docfunc)
     @istest
-    def modify_action_stringification(self, description, klass, name):
+    def modify_action_stringification(self, description, klass):
         args = [('target', 'a')]
-        if 'Chmod' in name:
+        if 'Chmod' in klass.__name__:
             args.append(('mode', '600'))
             act = klass('a', '600', self.file_context)
-        elif 'Chown' in name:
+        elif 'Chown' in klass.__name__:
             args.append(('user', 'user1'))
             args.append(('group', 'nogroup'))
             act = klass('a', 'user1', 'nogroup', self.file_context)
         else:
             assert False
 
-        if 'Dir' in name:
+        if 'Dir' in klass.__name__:
             args.append(('recursive', False))
 
         eq_(str(act), '{0}({1},context={2!r})'.format(
-            name, ','.join(['{0}={1}'.format(k, v) for (k, v) in args]),
+            klass.__name__,
+            ','.join(['{0}={1}'.format(k, v) for (k, v) in args]),
             self.file_context))

tests/unit/block/file_test.py

@@ -32,29 +32,30 @@ def make_file_block(mode='600', **kwargs):
     return assign_block_attrs(FileBlock(dummy_file_context),
                               mode=mode, **kwargs)
 
+fileblock_compilation_params = [
+    param('Unit: File Block Copy Compile',
+          ['backup', 'copy', 'chown_or_chmod', 'chown_or_chmod'], True, True),
+    param('Unit: File Block Create Compile',
+          ['create', 'chown_or_chmod', 'chown_or_chmod'],
+          False, True, action='create'),
+    param('Unit: File Block Copy Without User Skips Chown',
+          ['backup', 'copy', 'chmod'], True, True, user=None),
+    param('Unit: File Block Create Without User Skips Chown',
+          ['create', 'chmod'], True, False, action='create', user=None),
+    param('Unit: File Block Copy Without Group Skips Chown',
+          ['backup', 'copy', 'chmod'], True, False, group=None),
+    param('Unit: File Block Create Without Group Skips Chown',
+          ['create', 'chmod'], True, False, action='create', group=None),
+    param('Unit: File Block Copy Without Mode Skips Chmod',
+          ['backup', 'copy', 'chown'], False, False, mode=None),
+    param('Unit: File Block Create Without Mode Skips Chmod',
+          ['create', 'chown'], False, False, action='create', mode=None),
+]
+
 
 class TestWithScratchdir(ScratchWithExecCtx):
-    @parameterized.expand(
-        [param('Unit: File Block Copy Compile',
-               ['backup', 'copy', 'chown_or_chmod', 'chown_or_chmod'],
-               True, True),
-         param('Unit: File Block Create Compile',
-               ['create', 'chown_or_chmod', 'chown_or_chmod'],
-               False, True, action='create'),
-         param('Unit: File Block Copy Without User Skips Chown',
-               ['backup', 'copy', 'chmod'], True, True, user=None),
-         param('Unit: File Block Create Without User Skips Chown',
-               ['create', 'chmod'], True, False, action='create', user=None),
-         param('Unit: File Block Copy Without Group Skips Chown',
-               ['backup', 'copy', 'chmod'], True, False, group=None),
-         param('Unit: File Block Create Without Group Skips Chown',
-               ['create', 'chmod'], True, False, action='create', group=None),
-         param('Unit: File Block Copy Without Mode Skips Chmod',
-               ['backup', 'copy', 'chown'], False, False, mode=None),
-         param('Unit: File Block Create Without Mode Skips Chmod',
-               ['create', 'chown'], False, False, action='create', mode=None),
-         ],
-        testcase_func_doc=first_param_docfunc)
+    @parameterized.expand(fileblock_compilation_params,
+                          testcase_func_doc=first_param_docfunc)
     @istest
     def check_against_defaults(self, description, expected_al,
                                isroot, fexists, **kwargs):

tests/unit/parser/parse_test.py

@@ -1,5 +1,7 @@
 from nose.tools import istest, eq_, ok_
-from tests.framework import ensure_except, full_path, MockedGlobals
+from nose_parameterized import parameterized, param
+from tests.framework import (ensure_except, full_path, MockedGlobals,
+                             first_param_docfunc)
 
 from salve import paths
 from salve.parser import parse, Token
@@ -45,25 +47,47 @@ def _check_blocks(block_list, *args):
         eq_(len(block_list[i].attrs), num_attrs)
 
 
+basic_filecheck_params = [
+    param('Unit: Parser Empty File', 'empty.manifest'),
+    param('Unit: Parser Empty Block In File', 'empty_block.manifest',
+          (FileBlock, 0)),
+    param('Unit: Parser Attribute With Spaces', 'spaced_attr.manifest',
+          (FileBlock, 2)),
+]
+
+
+parse_exception_token_params = [
+    param('Unit: Parser Invalid Block Identifier From File',
+          ('invalid_block_id', Token.types.IDENTIFIER)),
+    param('Unit: Parser Unexpected Token', ('{', Token.types.BLOCK_START)),
+    param('Unit: Parser Partial Block Fails (No Open)',
+          ('file', Token.types.IDENTIFIER)),
+    param('Unit: Parser Partial Block Fails (No Close)',
+          ('file', Token.types.IDENTIFIER), ('{', Token.types.BLOCK_START)),
+    param('Unit: Parser Unassigned Attribute Fails',
+          ('file', Token.types.IDENTIFIER), ('{', Token.types.BLOCK_START),
+          ('source', Token.types.IDENTIFIER), ('}', Token.types.BLOCK_END)),
+]
+
+
+parse_exception_file_params = [
+    param('Unit: Parser Invalid Block Identifier',
+          'invalid_block_id.manifest'),
+]
+
+
 class TestParsingMockedGlobals(MockedGlobals):
+    @parameterized.expand(parse_exception_token_params,
+                          testcase_func_doc=first_param_docfunc)
     @istest
-    def invalid_block_id(self):
-        """
-        Unit: Parser Invalid Block Identifier
-        Verifies that attempting to parse a token stream containing an
-        unknown block identifier raises a ParsingException.
-        """
-        ensure_ParsingException(tokens=_generate_tokens(
-            ('invalid_block_id', Token.types.IDENTIFIER)))
+    def invalid_token_set(self, description, *args):
+        ensure_ParsingException(tokens=_generate_tokens(*args))
 
+    @parameterized.expand(parse_exception_file_params,
+                          testcase_func_doc=first_param_docfunc)
     @istest
-    def invalid_block_id_from_file(self):
-        """
-        Unit: Parser Invalid Block Identifier From File
-        Verifies that attempting to parse a file containing an unknown
-        block identifier raises a ParsingException.
-        """
-        ensure_ParsingException(filename='invalid_block_id.manifest')
+    def invalid_file(self, description, filename):
+        ensure_ParsingException(filename=filename)
 
     @istest
     def empty_token_list(self):
@@ -74,48 +98,6 @@ def empty_token_list(self):
         """
         eq_(len(parse.parse_tokens([])), 0)
 
-    @istest
-    def unexpected_token(self):
-        """
-        Unit: Parser Unexpected Token
-        Checks that parsing a token list with a token that violates the
-        SALVE grammar raises a ParsingException.
-        """
-        ensure_ParsingException(tokens=_generate_tokens(
-            ('{', Token.types.BLOCK_START)))
-
-    @istest
-    def unclosed_block1(self):
-        """
-        Unit: Parser Partial Block Fails (No Open)
-        Checks that parsing a token list with an unclosed block raises a
-        ParsingException.
-        """
-        ensure_ParsingException(tokens=_generate_tokens(
-            ('file', Token.types.IDENTIFIER)))
-
-    @istest
-    def unclosed_block2(self):
-        """
-        Unit: Parser Partial Block Fails (No Close)
-        Checks that parsing a token list with an unclosed block raises a
-        ParsingException.
-        """
-        ensure_ParsingException(tokens=_generate_tokens(
-            ('file', Token.types.IDENTIFIER), ('{', Token.types.BLOCK_START)))
-
-    @istest
-    def unassigned_attr(self):
-        """
-        Unit: Parser Unassigned Attribute Fails
-        Checks that parsing a block with an attribute that is declared but
-        is followed by a block close raises a ParsingException.
-        """
-        ensure_ParsingException(tokens=_generate_tokens(
-            ('file', Token.types.IDENTIFIER), ('{', Token.types.BLOCK_START),
-            ('source', Token.types.IDENTIFIER), ('}', Token.types.BLOCK_END)
-        ))
-
     @istest
     def empty_block(self):
         """
@@ -160,33 +142,11 @@ def multiple_attr_block(self):
         eq_(blocks[0]['source'], '/tmp/txt')
         eq_(blocks[0]['target'], '/tmp/txt2')
 
+    @parameterized.expand(basic_filecheck_params,
+                          testcase_func_doc=first_param_docfunc)
     @istest
-    def empty_manifest(self):
-        """
-        Unit: Parser Empty File
-        Checks that parsing an empty file produces an empty block list.
-        """
-        blocks = parse_filename(full_path('empty.manifest'))
-        _check_blocks(blocks)
-
-    @istest
-    def empty_block_in_file(self):
-        """
-        Unit: Parser Empty Block In File
-        Checks that parsing a file with an empty block is valid.
-        """
-        blocks = parse_filename(full_path('empty_block.manifest'))
-        _check_blocks(blocks, (FileBlock, 0))
-
-    @istest
-    def attribute_with_spaces(self):
-        """
-        Unit: Parser Attribute With Spaces
-        Checks that parsing an attribute that contains spaces in quotes
-        does not raise an error and correctly assigns to the attribute.
-        """
-        blocks = parse_filename(full_path('spaced_attr.manifest'))
-        _check_blocks(blocks, (FileBlock, 2))
+    def basic_fileparse_checks(self, description, filename, *args):
+        _check_blocks(parse_filename(full_path(filename)), *args)
 
     @istest
     def file_primary_attr_assigned(self):