From 527fd12f6a756b1d6a8b607334de28a117f02097 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 30 Nov 2015 00:01:17 -0600 Subject: [PATCH] Simplify more tests with parameterization Repetitive parser tests were repetitive. Only small amounts of cleverness makes them non-repetitive. --- salve/action/base.py | 2 +- salve/action/copy/base.py | 2 - tests/unit/action/modify_test.py | 32 ++++---- tests/unit/block/file_test.py | 43 ++++++----- tests/unit/parser/parse_test.py | 128 +++++++++++-------------------- 5 files changed, 83 insertions(+), 124 deletions(-) diff --git a/salve/action/base.py b/salve/action/base.py index e3b0848..5a44165 100644 --- a/salve/action/base.py +++ b/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 diff --git a/salve/action/copy/base.py b/salve/action/copy/base.py index 3ef1bb8..4e13b82 100644 --- a/salve/action/copy/base.py +++ b/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( diff --git a/tests/unit/action/modify_test.py b/tests/unit/action/modify_test.py index 55e948c..b6a2cf6 100644 --- a/tests/unit/action/modify_test.py +++ b/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)) diff --git a/tests/unit/block/file_test.py b/tests/unit/block/file_test.py index c54bf0b..78504ce 100644 --- a/tests/unit/block/file_test.py +++ b/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): diff --git a/tests/unit/parser/parse_test.py b/tests/unit/parser/parse_test.py index 703b106..2771e25 100644 --- a/tests/unit/parser/parse_test.py +++ b/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):