From 8fea7e90484917aa06e9be510a9590dfbfc1ae2d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 21 May 2019 11:04:58 +0200 Subject: [PATCH 1/8] Use byte comparison for contains I don't think there's any drawback open this in byte mode. Fixes https://github.com/galaxyproject/galaxy/issues/7957#issuecomment-493954901 --- lib/galaxy/tools/verify/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index f78d43b7f4f..2ed6e54fdb0 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -303,13 +303,13 @@ def files_re_match_multiline(file1, file2, attributes=None): def files_contains(file1, file2, attributes=None): """Check the contents of file2 for substrings found in file1, on a per-line basis.""" - local_file = io.open(file1, encoding='utf-8').readlines() # regex file + local_file = open(file1, 'rb').readlines() # regex file # TODO: allow forcing ordering of contains - history_data = io.open(file2, encoding='utf-8').read() + history_data = open(file2, 'rb').read() lines_diff = int(attributes.get('lines_diff', 0)) line_diff_count = 0 while local_file: - contains = local_file.pop(0).rstrip('\n\r') + contains = local_file.pop(0).rstrip(b'\n\r') if contains not in history_data: line_diff_count += 1 if line_diff_count > lines_diff: From ab7a38631e15c0c326b210a63926c9642f3f6159 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 22 May 2019 14:34:28 +0200 Subject: [PATCH 2/8] Unit test verify module --- test/unit/test_verify.py | 74 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 test/unit/test_verify.py diff --git a/test/unit/test_verify.py b/test/unit/test_verify.py new file mode 100644 index 00000000000..e325471647b --- /dev/null +++ b/test/unit/test_verify.py @@ -0,0 +1,74 @@ +import collections +import os +import tempfile + +import pytest + +from galaxy.tools.verify import ( + files_contains, + files_diff, + files_re_match, + files_re_match_multiline, +) + + +F1 = b"A\nB\nC" +F2 = b"A\nB\nD\nE" +F3 = b"A\nB\nD\n\xfc" +MULTINE_MATCH = b".*" +TestFile = collections.namedtuple('TestFile', 'value path') + + +def generate_tests(multiline=False): + files = [] + for b in [F1, F2, F3, MULTINE_MATCH]: + fd, path = tempfile.mkstemp() + with os.fdopen(fd, 'wb') as out: + out.write(b) + files.append(TestFile(b, path)) + f1, f2, f3, multiline_match = files + if multiline: + tests = [(multiline_match, f1, {'lines_diff': 0, 'sort': True}, None)] + else: + tests = [(f1, f1, {'lines_diff': 0}, None)] + tests.extend([ + (f1, f2, {'lines_diff': 0}, AssertionError), + (f1, f3, {'lines_diff': 0}, AssertionError), + ]) + return tests + + +@pytest.mark.parametrize('file1,file2,attributes,expect', generate_tests()) +def test_files_contains(file1, file2, attributes, expect): + if expect is not None: + with pytest.raises(expect): + files_contains(file1.path, file2.path, attributes) + else: + files_contains(file2.path, file2.path, attributes) + + +@pytest.mark.parametrize('file1,file2,attributes,expect', generate_tests()) +def test_files_diff(file1, file2, attributes, expect): + if expect is not None: + with pytest.raises(expect): + files_diff(file1.path, file2.path, attributes) + else: + files_diff(file1.path, file2.path, attributes) + + +@pytest.mark.parametrize('file1,file2,attributes,expect', generate_tests()) +def test_files_re_match(file1, file2, attributes, expect): + if expect is not None: + with pytest.raises(expect): + files_re_match(file1.path, file2.path, attributes) + else: + files_re_match(file1.path, file2.path, attributes) + + +@pytest.mark.parametrize('file1,file2,attributes,expect', generate_tests(multiline=True)) +def test_files_re_match_multiline(file1, file2, attributes, expect): + if expect is not None: + with pytest.raises(expect): + files_re_match_multiline(file1.path, file2.path, attributes) + else: + files_re_match_multiline(file1.path, file2.path, attributes) From 30bec0028dc79e5e2e9d3a117f2908c47506be5a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 22 May 2019 14:35:01 +0200 Subject: [PATCH 3/8] Use bytestring comparisons in verify module --- lib/galaxy/tools/verify/__init__.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index 2ed6e54fdb0..9f8a55fa4df 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -3,7 +3,6 @@ import difflib import filecmp import hashlib -import io import logging import os import os.path @@ -268,9 +267,9 @@ def files_diff(file1, file2, attributes=None): def files_re_match(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match.""" - local_file = io.open(file1, encoding='utf-8').readlines() # regex file - history_data = io.open(file2, encoding='utf-8').readlines() - assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), ''.join(history_data[:40])) + local_file = open(file1, 'rb').readlines() # regex file + history_data = open(file2, 'rb').readlines() + assert len(local_file) == len(history_data), b'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), b''.join(history_data[:40])) if attributes is None: attributes = {} if attributes.get('sort', False): @@ -279,24 +278,24 @@ def files_re_match(file1, file2, attributes=None): line_diff_count = 0 diffs = [] for i in range(len(history_data)): - if not re.match(local_file[i].rstrip('\r\n'), history_data[i].rstrip('\r\n')): + if not re.match(local_file[i].rstrip(b'\r\n'), history_data[i].rstrip(b'\r\n')): line_diff_count += 1 - diffs.append('Regular Expression: %s\nData file : %s' % (local_file[i].rstrip('\r\n'), history_data[i].rstrip('\r\n'))) + diffs.append(b'Regular Expression: %s\nData file : %s' % (local_file[i].rstrip(b'\r\n'), history_data[i].rstrip(b'\r\n'))) if line_diff_count > lines_diff: - raise AssertionError("Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, "".join(diffs))) + raise AssertionError(b"Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, b"".join(diffs))) def files_re_match_multiline(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match in multiline mode.""" - local_file = io.open(file1, encoding='utf-8').read() # regex file + local_file = open(file1, 'rb').read() # regex file if attributes is None: attributes = {} if attributes.get('sort', False): - history_data = io.open(file2, encoding='utf-8').readlines() + history_data = open(file2, 'rb').readlines() history_data.sort() - history_data = ''.join(history_data) + history_data = b''.join(history_data) else: - history_data = io.open(file2, encoding='utf-8').read() + history_data = open(file2, 'rb').read() # lines_diff not applicable to multiline matching assert re.match(local_file, history_data, re.MULTILINE), "Multiline Regular expression did not match data file" From 9f6bc89d69e0b4fe0d7fe86cffecad1c77ab736b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 22 May 2019 17:59:49 +0200 Subject: [PATCH 4/8] Do non-shallow file comparison --- lib/galaxy/tools/verify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index 9f8a55fa4df..5cc4830866e 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -195,7 +195,7 @@ def files_diff(file1, file2, attributes=None): count += 1 return count - if not filecmp.cmp(file1, file2): + if not filecmp.cmp(file1, file2, shallow=False): if attributes is None: attributes = {} decompress = attributes.get("decompress", None) From 2d71b0bdfe8a234d28ceb90dcd85ad41384ecdfe Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 23 May 2019 11:07:06 +0200 Subject: [PATCH 5/8] Use io.open with errors="repalce" instead of comparing byte strings --- lib/galaxy/tools/verify/__init__.py | 54 +++++++++++++---------------- test/unit/test_verify.py | 14 ++++---- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index 5cc4830866e..6e873818b28 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -3,6 +3,7 @@ import difflib import filecmp import hashlib +import io import logging import os import os.path @@ -15,6 +16,7 @@ try: except ImportError: pysam = None +from galaxy.util import unicodify from galaxy.util.compression_utils import get_fileobj from .asserts import verify_assertions from .test_data import TestDataResolver @@ -109,9 +111,9 @@ def verify( log.error(error_log_msg, exc_info=True) else: log.debug('## GALAXY_TEST_SAVE=%s. saved %s' % (keep_outputs_dir, ofn)) + compare = attributes.get('compare', 'diff') try: - compare = attributes.get('compare', 'diff') - if attributes.get('ftype', None) in ['bam', 'qname_sorted.bam', 'qname_input_sorted.bam', 'unsorted.bam']: + if attributes.get('ftype', None) in ['bam', 'qname_sorted.bam', 'qname_input_sorted.bam', 'unsorted.bam', 'cram']: try: local_fh, temp_name = _bam_to_sam(local_name, temp_name) local_name = local_fh.name @@ -204,17 +206,9 @@ def files_diff(file1, file2, attributes=None): compressed_formats = None else: compressed_formats = [] - is_pdf = False - try: - local_file = get_fileobj(file1, compressed_formats=compressed_formats).readlines() - history_data = get_fileobj(file2, compressed_formats=compressed_formats).readlines() - except UnicodeDecodeError: - if file1.endswith('.pdf') or file2.endswith('.pdf'): - is_pdf = True - local_file = open(file1, 'rb').readlines() - history_data = open(file2, 'rb').readlines() - else: - raise AssertionError("Binary data detected, not displaying diff") + local_file = [unicodify(l) for l in get_fileobj(file1, mode='rb', compressed_formats=compressed_formats)] + history_data = [unicodify(l) for l in get_fileobj(file2, mode='rb', compressed_formats=compressed_formats)] + is_pdf = file1.endswith('.pdf') or file2.endswith('.pdf') if attributes.get('sort', False): local_file.sort() history_data.sort() @@ -267,9 +261,9 @@ def files_diff(file1, file2, attributes=None): def files_re_match(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match.""" - local_file = open(file1, 'rb').readlines() # regex file - history_data = open(file2, 'rb').readlines() - assert len(local_file) == len(history_data), b'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), b''.join(history_data[:40])) + local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').readlines() # regex file + history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').readlines() + assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), ''.join(history_data[:40])) if attributes is None: attributes = {} if attributes.get('sort', False): @@ -277,38 +271,40 @@ def files_re_match(file1, file2, attributes=None): lines_diff = int(attributes.get('lines_diff', 0)) line_diff_count = 0 diffs = [] - for i in range(len(history_data)): - if not re.match(local_file[i].rstrip(b'\r\n'), history_data[i].rstrip(b'\r\n')): + for regex_line, data_line in zip(local_file, history_data): + if not re.match(regex_line.rstrip('\r\n'), data_line.rstrip('\r\n')): line_diff_count += 1 - diffs.append(b'Regular Expression: %s\nData file : %s' % (local_file[i].rstrip(b'\r\n'), history_data[i].rstrip(b'\r\n'))) - if line_diff_count > lines_diff: - raise AssertionError(b"Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, b"".join(diffs))) + diffs.append('Regular Expression: %s, Data file: %s\n' % (regex_line.rstrip('\r\n'), data_line.rstrip('\r\n'))) + if line_diff_count > lines_diff: + raise AssertionError("Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, "".join(diffs))) def files_re_match_multiline(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match in multiline mode.""" - local_file = open(file1, 'rb').read() # regex file + local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').read() # regex file if attributes is None: attributes = {} if attributes.get('sort', False): - history_data = open(file2, 'rb').readlines() + history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').readlines() history_data.sort() - history_data = b''.join(history_data) + history_data = ''.join(history_data) else: - history_data = open(file2, 'rb').read() + history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').read() # lines_diff not applicable to multiline matching assert re.match(local_file, history_data, re.MULTILINE), "Multiline Regular expression did not match data file" def files_contains(file1, file2, attributes=None): """Check the contents of file2 for substrings found in file1, on a per-line basis.""" - local_file = open(file1, 'rb').readlines() # regex file + if attributes is None: + attributes = {} + local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').readlines() # regex file # TODO: allow forcing ordering of contains - history_data = open(file2, 'rb').read() + history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').read() lines_diff = int(attributes.get('lines_diff', 0)) line_diff_count = 0 - while local_file: - contains = local_file.pop(0).rstrip(b'\n\r') + for contains in local_file: + contains = contains.rstrip('\r\n') if contains not in history_data: line_diff_count += 1 if line_diff_count > lines_diff: diff --git a/test/unit/test_verify.py b/test/unit/test_verify.py index e325471647b..2dddb543135 100644 --- a/test/unit/test_verify.py +++ b/test/unit/test_verify.py @@ -13,16 +13,16 @@ from galaxy.tools.verify import ( F1 = b"A\nB\nC" -F2 = b"A\nB\nD\nE" -F3 = b"A\nB\nD\n\xfc" +F2 = b"A\nB\nD\nE" * 61 +F3 = b"A\nB\n\xfc" MULTINE_MATCH = b".*" TestFile = collections.namedtuple('TestFile', 'value path') def generate_tests(multiline=False): files = [] - for b in [F1, F2, F3, MULTINE_MATCH]: - fd, path = tempfile.mkstemp() + for b, ext in [(F1, '.txt'), (F2, '.txt'), (F3, '.pdf'), (MULTINE_MATCH, '.txt')]: + fd, path = tempfile.mkstemp(suffix=ext) with os.fdopen(fd, 'wb') as out: out.write(b) files.append(TestFile(b, path)) @@ -30,10 +30,10 @@ def generate_tests(multiline=False): if multiline: tests = [(multiline_match, f1, {'lines_diff': 0, 'sort': True}, None)] else: - tests = [(f1, f1, {'lines_diff': 0}, None)] + tests = [(f1, f1, {'lines_diff': 0, 'sort': True}, None)] tests.extend([ - (f1, f2, {'lines_diff': 0}, AssertionError), - (f1, f3, {'lines_diff': 0}, AssertionError), + (f1, f2, {'lines_diff': 0, 'sort': True}, AssertionError), + (f1, f3, None, AssertionError), ]) return tests From a398127ede922d504ec7f1c39e66df22a9f0a16d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 23 May 2019 13:43:57 +0200 Subject: [PATCH 6/8] Work with bytestrings in `files_contains`, `files_re_match_*` But read utf-8 unicode in `lines_diff`. This will replace non-utf8 characters and seems like a reasonable tradeoff for text files mixed with byte contents, like pdfs. --- lib/galaxy/tools/verify/__init__.py | 31 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index 6e873818b28..a58cb39bcf2 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -3,7 +3,6 @@ import difflib import filecmp import hashlib -import io import logging import os import os.path @@ -206,6 +205,9 @@ def files_diff(file1, file2, attributes=None): compressed_formats = None else: compressed_formats = [] + + # Open expected contents and history data in binary mode and run lines through unicodify, + # which will replace non utf-8 characters. local_file = [unicodify(l) for l in get_fileobj(file1, mode='rb', compressed_formats=compressed_formats)] history_data = [unicodify(l) for l in get_fileobj(file2, mode='rb', compressed_formats=compressed_formats)] is_pdf = file1.endswith('.pdf') or file2.endswith('.pdf') @@ -261,9 +263,9 @@ def files_diff(file1, file2, attributes=None): def files_re_match(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match.""" - local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').readlines() # regex file - history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').readlines() - assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), ''.join(history_data[:40])) + local_file = open(file1, 'rb').readlines() # regex file + history_data = open(file2, 'rb').readlines() + assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), ''.join(unicodify(history_data[:40]))) if attributes is None: attributes = {} if attributes.get('sort', False): @@ -272,24 +274,25 @@ def files_re_match(file1, file2, attributes=None): line_diff_count = 0 diffs = [] for regex_line, data_line in zip(local_file, history_data): - if not re.match(regex_line.rstrip('\r\n'), data_line.rstrip('\r\n')): + if not re.match(regex_line.rstrip(b'\r\n'), data_line.rstrip(b'\r\n')): line_diff_count += 1 - diffs.append('Regular Expression: %s, Data file: %s\n' % (regex_line.rstrip('\r\n'), data_line.rstrip('\r\n'))) + diffs.append('Regular Expression: %s, Data file: %s\n' % (unicodify(regex_line).rstrip('\r\n'), + unicodify(data_line).rstrip('\r\n'))) if line_diff_count > lines_diff: raise AssertionError("Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, "".join(diffs))) def files_re_match_multiline(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match in multiline mode.""" - local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').read() # regex file + local_file = open(file1, 'rb').read() # regex file if attributes is None: attributes = {} if attributes.get('sort', False): - history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').readlines() + history_data = open(file2, 'rb').readlines() history_data.sort() - history_data = ''.join(history_data) + history_data = b''.join(history_data) else: - history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').read() + history_data = open(file2, 'rb').read() # lines_diff not applicable to multiline matching assert re.match(local_file, history_data, re.MULTILINE), "Multiline Regular expression did not match data file" @@ -298,14 +301,14 @@ def files_contains(file1, file2, attributes=None): """Check the contents of file2 for substrings found in file1, on a per-line basis.""" if attributes is None: attributes = {} - local_file = io.open(file1, 'r', encoding='utf-8', errors='replace').readlines() # regex file + local_file = open(file1, 'rb').readlines() # regex file # TODO: allow forcing ordering of contains - history_data = io.open(file2, 'r', encoding='utf-8', errors='replace').read() + history_data = open(file2, 'rb').read() lines_diff = int(attributes.get('lines_diff', 0)) line_diff_count = 0 for contains in local_file: - contains = contains.rstrip('\r\n') + contains = contains.rstrip(b'\r\n') if contains not in history_data: line_diff_count += 1 if line_diff_count > lines_diff: - raise AssertionError("Failed to find '%s' in history data. (lines_diff=%i):\n" % (contains, lines_diff)) + raise AssertionError("Failed to find '%s' in history data. (lines_diff=%i):\n" % (unicodify(contains), lines_diff)) From d9b0f5490668bec16fa7b3f1e6f875fa2f09430b Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Sun, 26 May 2019 14:55:14 +0100 Subject: [PATCH 7/8] Try to verify tool test output first as Unicode --- lib/galaxy/tools/verify/__init__.py | 85 +++++++++++++++++++++-------- test/unit/test_verify.py | 4 +- 2 files changed, 65 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index a58cb39bcf2..b95fed8c46b 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -3,6 +3,7 @@ import difflib import filecmp import hashlib +import io import logging import os import os.path @@ -205,12 +206,21 @@ def files_diff(file1, file2, attributes=None): compressed_formats = None else: compressed_formats = [] - - # Open expected contents and history data in binary mode and run lines through unicodify, - # which will replace non utf-8 characters. - local_file = [unicodify(l) for l in get_fileobj(file1, mode='rb', compressed_formats=compressed_formats)] - history_data = [unicodify(l) for l in get_fileobj(file2, mode='rb', compressed_formats=compressed_formats)] - is_pdf = file1.endswith('.pdf') or file2.endswith('.pdf') + is_pdf = False + try: + with get_fileobj(file2, compressed_formats=compressed_formats) as fh: + history_data = fh.readlines() + with get_fileobj(file1, compressed_formats=compressed_formats) as fh: + local_file = fh.readlines() + except UnicodeDecodeError: + if file1.endswith('.pdf') or file2.endswith('.pdf'): + is_pdf = True + # Replace non-Unicode characters using unicodify(), + # difflib.unified_diff doesn't work on list of bytes + history_data = [unicodify(l) for l in get_fileobj(file2, mode='rb', compressed_formats=compressed_formats)] + local_file = [unicodify(l) for l in get_fileobj(file1, mode='rb', compressed_formats=compressed_formats)] + else: + raise AssertionError("Binary data detected, not displaying diff") if attributes.get('sort', False): local_file.sort() history_data.sort() @@ -263,9 +273,21 @@ def files_diff(file1, file2, attributes=None): def files_re_match(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match.""" - local_file = open(file1, 'rb').readlines() # regex file - history_data = open(file2, 'rb').readlines() - assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), ''.join(unicodify(history_data[:40]))) + join_char = '' + to_strip = os.linesep + try: + with io.open(file2, encoding='utf-8') as fh: + history_data = fh.readlines() + with io.open(file1, encoding='utf-8') as fh: + local_file = fh.readlines() + except UnicodeDecodeError: + join_char = b'' + to_strip = os.linesep.encode('utf-8') + with open(file2, 'rb') as fh: + history_data = fh.readlines() + with open(file1, 'rb') as fh: + local_file = fh.readlines() + assert len(local_file) == len(history_data), 'Data File and Regular Expression File contain a different number of lines (%d != %d)\nHistory Data (first 40 lines):\n%s' % (len(local_file), len(history_data), join_char.join(history_data[:40])) if attributes is None: attributes = {} if attributes.get('sort', False): @@ -274,41 +296,60 @@ def files_re_match(file1, file2, attributes=None): line_diff_count = 0 diffs = [] for regex_line, data_line in zip(local_file, history_data): - if not re.match(regex_line.rstrip(b'\r\n'), data_line.rstrip(b'\r\n')): + regex_line = regex_line.rstrip(to_strip) + data_line = data_line.rstrip(to_strip) + if not re.match(regex_line, data_line): line_diff_count += 1 - diffs.append('Regular Expression: %s, Data file: %s\n' % (unicodify(regex_line).rstrip('\r\n'), - unicodify(data_line).rstrip('\r\n'))) + diffs.append('Regular Expression: %s, Data file: %s\n' % (regex_line, data_line)) if line_diff_count > lines_diff: raise AssertionError("Regular expression did not match data file (allowed variants=%i):\n%s" % (lines_diff, "".join(diffs))) def files_re_match_multiline(file1, file2, attributes=None): """Check the contents of 2 files for differences using re.match in multiline mode.""" - local_file = open(file1, 'rb').read() # regex file + join_char = '' + try: + with io.open(file2, encoding='utf-8') as fh: + history_data = fh.readlines() + with io.open(file1, encoding='utf-8') as fh: + local_file = fh.read() + except UnicodeDecodeError: + join_char = b'' + with open(file2, 'rb') as fh: + history_data = fh.readlines() + with open(file1, 'rb') as fh: + local_file = fh.read() if attributes is None: attributes = {} if attributes.get('sort', False): - history_data = open(file2, 'rb').readlines() history_data.sort() - history_data = b''.join(history_data) - else: - history_data = open(file2, 'rb').read() + history_data = join_char.join(history_data) # lines_diff not applicable to multiline matching assert re.match(local_file, history_data, re.MULTILINE), "Multiline Regular expression did not match data file" def files_contains(file1, file2, attributes=None): """Check the contents of file2 for substrings found in file1, on a per-line basis.""" + # TODO: allow forcing ordering of contains + to_strip = os.linesep + try: + with io.open(file2, encoding='utf-8') as fh: + history_data = fh.read() + with io.open(file1, encoding='utf-8') as fh: + local_file = fh.readlines() + except UnicodeDecodeError: + to_strip = os.linesep.encode('utf-8') + with open(file2, 'rb') as fh: + history_data = fh.read() + with open(file1, 'rb') as fh: + local_file = fh.readlines() if attributes is None: attributes = {} - local_file = open(file1, 'rb').readlines() # regex file - # TODO: allow forcing ordering of contains - history_data = open(file2, 'rb').read() lines_diff = int(attributes.get('lines_diff', 0)) line_diff_count = 0 for contains in local_file: - contains = contains.rstrip(b'\r\n') + contains = contains.rstrip(to_strip) if contains not in history_data: line_diff_count += 1 if line_diff_count > lines_diff: - raise AssertionError("Failed to find '%s' in history data. (lines_diff=%i):\n" % (unicodify(contains), lines_diff)) + raise AssertionError("Failed to find '%s' in history data. (lines_diff=%i):\n" % (contains, lines_diff)) diff --git a/test/unit/test_verify.py b/test/unit/test_verify.py index 2dddb543135..ac78103aa73 100644 --- a/test/unit/test_verify.py +++ b/test/unit/test_verify.py @@ -15,13 +15,13 @@ from galaxy.tools.verify import ( F1 = b"A\nB\nC" F2 = b"A\nB\nD\nE" * 61 F3 = b"A\nB\n\xfc" -MULTINE_MATCH = b".*" +MULTILINE_MATCH = b".*" TestFile = collections.namedtuple('TestFile', 'value path') def generate_tests(multiline=False): files = [] - for b, ext in [(F1, '.txt'), (F2, '.txt'), (F3, '.pdf'), (MULTINE_MATCH, '.txt')]: + for b, ext in [(F1, '.txt'), (F2, '.txt'), (F3, '.pdf'), (MULTILINE_MATCH, '.txt')]: fd, path = tempfile.mkstemp(suffix=ext) with os.fdopen(fd, 'wb') as out: out.write(b) From 62f5572a430df6457038ae2b50e2e0278d055262 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 27 May 2019 11:35:20 +0200 Subject: [PATCH 8/8] Add carriage return unit test --- test/unit/test_verify.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/test_verify.py b/test/unit/test_verify.py index ac78103aa73..8de580cec5b 100644 --- a/test/unit/test_verify.py +++ b/test/unit/test_verify.py @@ -15,18 +15,19 @@ from galaxy.tools.verify import ( F1 = b"A\nB\nC" F2 = b"A\nB\nD\nE" * 61 F3 = b"A\nB\n\xfc" +F4 = b"A\r\nB\nC" MULTILINE_MATCH = b".*" TestFile = collections.namedtuple('TestFile', 'value path') def generate_tests(multiline=False): files = [] - for b, ext in [(F1, '.txt'), (F2, '.txt'), (F3, '.pdf'), (MULTILINE_MATCH, '.txt')]: + for b, ext in [(F1, '.txt'), (F2, '.txt'), (F3, '.pdf'), (F4, '.txt'), (MULTILINE_MATCH, '.txt')]: fd, path = tempfile.mkstemp(suffix=ext) with os.fdopen(fd, 'wb') as out: out.write(b) files.append(TestFile(b, path)) - f1, f2, f3, multiline_match = files + f1, f2, f3, f4, multiline_match = files if multiline: tests = [(multiline_match, f1, {'lines_diff': 0, 'sort': True}, None)] else: @@ -34,6 +35,7 @@ def generate_tests(multiline=False): tests.extend([ (f1, f2, {'lines_diff': 0, 'sort': True}, AssertionError), (f1, f3, None, AssertionError), + (f1, f4, None, None), ]) return tests