From df6e535f05a7a04d0a7c02845bdd52f44966ecf4 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Fri, 13 Apr 2018 11:25:24 -0500 Subject: [PATCH 1/2] Fix diffing binary files in file.get_diff --- salt/modules/file.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/salt/modules/file.py b/salt/modules/file.py index 97d6ca1a34..57e045cc30 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -4969,9 +4969,8 @@ def get_diff(file1, args = [] for idx, filename in enumerate(files): try: - with salt.utils.files.fopen(filename, 'r') as fp_: - args.append([salt.utils.stringutils.to_unicode(x) - for x in fp_.readlines()]) + with salt.utils.files.fopen(filename, 'rb') as fp_: + args.append(fp_.readlines()) except (IOError, OSError) as exc: raise CommandExecutionError( 'Failed to read {0}: {1}'.format( @@ -4991,15 +4990,13 @@ def get_diff(file1, ret = bdiff else: if show_filenames: - args.extend( - [salt.utils.stringutils.to_unicode(x) for x in files] + args.extend(files) + ret = ''.join( + difflib.unified_diff( + *salt.utils.data.decode(args) ) - ret = salt.utils.locales.sdecode( - ''.join(difflib.unified_diff(*args)) # pylint: disable=no-value-for-parameter ) - return ret - - return '' + return ret def manage_file(name, From 87f6cefea31389ec9c2179bdd1c4e9fc6cc10b25 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 17 Apr 2018 15:15:53 -0500 Subject: [PATCH 2/2] Rewrite flaky utf8 state to make it easier to troubleshoot --- tests/integration/states/test_file.py | 179 ++++++++++---------------- 1 file changed, 66 insertions(+), 113 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 5963b4a5c3..0fbfd0f1b9 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -2176,6 +2176,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): This is more generic than just a file test. Feel free to move ''' + self.maxDiff = None korean_1 = '한국어 시험' korean_2 = '첫 번째 행' korean_3 = '마지막 행' @@ -2187,58 +2188,40 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): test_file_encoded = test_file template_path = os.path.join(TMP_STATE_TREE, 'issue-8947.sls') # create the sls template - template_lines = [ - '# -*- coding: utf-8 -*-', - 'some-utf8-file-create:', - ' file.managed:', - " - name: '{0}'".format(test_file), - ' - contents: |', - ' {0}'.format(korean_1), - ' - makedirs: True', - ' - replace: True', - ' - show_diff: True', - 'some-utf8-file-create2:', - ' file.managed:', - " - name: '{0}'".format(test_file), - ' - contents: |', - ' {0}'.format(korean_2), - ' {0}'.format(korean_1), - ' {0}'.format(korean_3), - ' - replace: True', - ' - show_diff: True', - 'some-utf8-file-exists:', - ' file.exists:', - " - name: '{0}'".format(test_file), - ' - require:', - ' - file: some-utf8-file-create2', - 'some-utf8-file-content-test:', - ' cmd.run:', - ' - name: \'cat "{0}"\''.format(test_file), - ' - require:', - ' - file: some-utf8-file-exists', - 'some-utf8-file-content-remove:', - ' cmd.run:', - ' - name: \'rm -f "{0}"\''.format(test_file), - ' - require:', - ' - cmd: some-utf8-file-content-test', - 'some-utf8-file-removed:', - ' file.missing:', - " - name: '{0}'".format(test_file), - ' - require:', - ' - cmd: some-utf8-file-content-remove', - ] + template = textwrap.dedent('''\ + some-utf8-file-create: + file.managed: + - name: {test_file} + - contents: {korean_1} + - makedirs: True + - replace: True + - show_diff: True + some-utf8-file-create2: + file.managed: + - name: {test_file} + - contents: | + {korean_2} + {korean_1} + {korean_3} + - replace: True + - show_diff: True + some-utf8-file-content-test: + cmd.run: + - name: 'cat "{test_file}"' + - require: + - file: some-utf8-file-create2 + '''.format(**locals())) + + # Save template file with salt.utils.files.fopen(template_path, 'wb') as fp_: - fp_.write( - salt.utils.stringutils.to_bytes( - os.linesep.join(template_lines) - ) - ) + fp_.write(salt.utils.stringutils.to_bytes(template)) + try: - ret = self.run_function('state.sls', mods='issue-8947') - if not isinstance(ret, dict): + result = self.run_function('state.sls', mods='issue-8947') + if not isinstance(result, dict): raise AssertionError( ('Something went really wrong while testing this sls:' - ' {0}').format(repr(ret)) + ' {0}').format(repr(result)) ) # difflib produces different output on python 2.6 than on >=2.7 if sys.version_info < (2, 7): @@ -2250,76 +2233,46 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): ' 한국어 시험\n' '+마지막 행\n' ) - # future_lint: disable=blacklisted-function - expected = { - 'file_|-some-utf8-file-create_|-{0}_|-managed'.format(test_file_encoded): { - 'name': test_file_encoded, - '__run_num__': 0, - 'comment': 'File {0} updated'.format(test_file_encoded), - 'diff': 'New file' - }, - 'file_|-some-utf8-file-create2_|-{0}_|-managed'.format(test_file_encoded): { - 'name': test_file_encoded, - '__run_num__': 1, - 'comment': 'File {0} updated'.format(test_file_encoded), - 'diff': diff - }, - 'file_|-some-utf8-file-exists_|-{0}_|-exists'.format(test_file_encoded): { - 'name': test_file_encoded, - '__run_num__': 2, - 'comment': 'Path {0} exists'.format(test_file_encoded) - }, - 'cmd_|-some-utf8-file-content-test_|-cat "{0}"_|-run'.format(test_file_encoded): { - 'name': 'cat "{0}"'.format(test_file_encoded), - '__run_num__': 3, - 'comment': 'Command "cat "{0}"" run'.format(test_file_encoded), - 'stdout': '{0}\n{1}\n{2}'.format( - korean_2, - korean_1, - korean_3, - ) - }, - 'cmd_|-some-utf8-file-content-remove_|-rm -f "{0}"_|-run'.format(test_file_encoded): { - 'name': 'rm -f "{0}"'.format(test_file_encoded), - '__run_num__': 4, - 'comment': 'Command "rm -f "{0}"" run'.format(test_file_encoded), - 'stdout': '' - }, - 'file_|-some-utf8-file-removed_|-{0}_|-missing'.format(test_file_encoded): { - 'name': test_file_encoded, - '__run_num__': 5, - 'comment': 'Path {0} is missing'.format(test_file_encoded), - } - } - # future_lint: enable=blacklisted-function - result = {} - for name, step in six.iteritems(ret): - self.assertSaltTrueReturn({name: step}) - result.update({ - name: { - 'name': step['name'], - '__run_num__': step['__run_num__'], - 'comment': step['comment'] - }}) - if 'diff' in step['changes']: - result[name]['diff'] = step['changes']['diff'] - if 'stdout' in step['changes']: - result[name]['stdout'] = step['changes']['stdout'] - self.maxDiff = None + ret = {x.split('_|-')[1]: y for x, y in six.iteritems(result)} - self.assertEqual(expected, result) - # future_lint: disable=blacklisted-function - cat_id = 'cmd_|-some-utf8-file-content-test_|-cat "{0}"_|-run'.format(test_file_encoded) - # future_lint: enable=blacklisted-function + # Confirm initial creation of file self.assertEqual( - salt.utils.stringutils.to_unicode(result[cat_id]['stdout']), - korean_2 + '\n' + korean_1 + '\n' + korean_3 + ret['some-utf8-file-create']['comment'], + 'File {0} updated'.format(test_file_encoded) ) + self.assertEqual( + ret['some-utf8-file-create']['changes'], + {'diff': 'New file'} + ) + + # Confirm file was modified and that the diff was as expected + self.assertEqual( + ret['some-utf8-file-create2']['comment'], + 'File {0} updated'.format(test_file_encoded) + ) + self.assertEqual( + ret['some-utf8-file-create2']['changes'], + {'diff': diff} + ) + + # Confirm that the file has the expected contents as specified in + # the prior state. + self.assertEqual( + ret['some-utf8-file-content-test']['comment'], + 'Command "cat "{0}"" run'.format(test_file_encoded) + ) + self.assertEqual( + ret['some-utf8-file-content-test']['changes']['stdout'], + '\n'.join((korean_2, korean_1, korean_3)) + ) + finally: - if os.path.isdir(test_file): - os.unlink(test_file) - os.unlink(template_path) + for path in (test_file, template_path): + try: + os.remove(path) + except OSError: + pass @skip_if_not_root @skipIf(not HAS_PWD, "pwd not available. Skipping test")