From 6e8e5747df26e896b6981ca551e0c821a3ce4a1f Mon Sep 17 00:00:00 2001 From: Martin Owens Date: Fri, 28 May 2021 18:38:36 -0400 Subject: [PATCH] Upgrade the testing export compare mechanisms In order to make test writing easier, this upgrade changes the way the EXPORT_COMPARE variable is used. It now comes in 4 modes EXPORT_COMPARE=0 (default) - Compare delete mode, deletes comparisons EXPORT_COMPARE=1 - Compare check mode, saves the comparison under a new name EXPORT_COMPARE=2 - Compare write mode, saves the comparison if it's new EXPORT_COMPARE=3 - Compare overwrite mode, this saves no matter what Other changes include: * New outfile for assertCompare which cleans some tests of having to do their own EXPORT_COMPARE mechanisms. * The use of compare_file_extension during compare check mode allows opening of the files in programs without as many issues as '.export' * Rerun the test when writing, so the test is still run even if the output file didn't exist. And double checks consistancy when overwriting. * _base_compare is now split out so it could be over-loaded by a test if they wanted to do their own fancy comparison. * gitignore will ignore all files that don't end in .out (fancy!) --- .gitignore | 5 +- inkex/tester/__init__.py | 118 +++++++++++++----- ...tedonly__False__--filepath__TMP_DIR__.out} | Bin ...mbeded_image01__--filepath__TMP_DIR__.out} | Bin tests/test_gcodetools.py | 29 +---- tests/test_guillotine.py | 8 +- tests/test_hershey.py | 16 +-- tests/test_image_extract.py | 25 +--- 8 files changed, 108 insertions(+), 93 deletions(-) rename tests/data/refs/{image_extract__--selectedonly__False__--filepath__TMP_DIR__img__.out => image_extract__--selectedonly__False__--filepath__TMP_DIR__.out} (100%) rename tests/data/refs/{image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__img__.out => image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__.out} (100%) diff --git a/.gitignore b/.gitignore index fae177ae0..aaca9e578 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,8 @@ old .pytest_cache .idea dist -*.out.export cov.xml coverage.xml -.vscode/ \ No newline at end of file +.vscode/ +tests/data/refs/* +!tests/data/refs/*.out diff --git a/inkex/tester/__init__.py b/inkex/tester/__init__.py index f896a6441..ba24c91c0 100644 --- a/inkex/tester/__init__.py +++ b/inkex/tester/__init__.py @@ -56,14 +56,25 @@ coverage score indicates that your test is better at exercising the various options, features, and branches within your code. Generating comparison output can be done using the EXPORT_COMPARE environment -variable when calling pytest. For example: +variable when calling pytest and comes in 3 modes, the first of which is the +CHECK comparisons mode: EXPORT_COMPARE=1 pytest tests/test_my_specific_test.py -This will create files in `tests/data/refs/*.out.export` and these files should -be manually checked to make sure they are correct before being renamed and stripped -of the `.export` suffix. pytest should then be re-run to confirm before -committing to the repository. +This will create files in `tests/data/refs/*.{ext}` and these files +should be manually checked to make sure they are correct. Once you are happy +with the output you can re-run the test with the WRITE comparisons mode: + + EXPORT_COMPARE=2 pytest tests/test_my_specific_test.py + +Which will create an output file of the right name and then run the test suite +against it. But only if the file doesn't already exist. The final mode is the +OVERWRITE comparisons mode: + + EXPORT_COMPARE=3 pytest tests/test_my_specific_test.py + +This is like mode 2, but will over-write any existing files too. This allows +you to update the test compare files. """ import os @@ -91,6 +102,7 @@ if False: # pylint: disable=using-constant-test from typing import Type, List from .filters import Compare +COMPARE_DELETE, COMPARE_CHECK, COMPARE_WRITE, COMPARE_OVERWRITE = range(4) class NoExtension(InkscapeExtension): # pylint: disable=too-few-public-methods """Test case must specify 'self.effect_class' to assertEffect.""" @@ -167,7 +179,7 @@ class TestCase(MockCommandMixin, BaseCase): return os.path.join(self.tempdir, filename) @classmethod - def data_file(cls, filename, *parts): + def data_file(cls, filename, *parts, check_exists=True): """Provide a data file from a filename, can accept directories as arguments.""" if os.path.isabs(filename): # Absolute root was passed in, so we trust that (it might be a tempdir) @@ -176,7 +188,7 @@ class TestCase(MockCommandMixin, BaseCase): # Otherwise we assume it's relative to the test data dir. full_path = os.path.join(cls.datadir(), filename, *parts) - if not os.path.isfile(full_path): + if not os.path.isfile(full_path) and check_exists: raise IOError(f"Can't find test data file: {full_path}") return full_path @@ -285,6 +297,14 @@ class ComparisonMixin: ('--id=p1', '--id=r3'), ] + compare_file_extension = 'svg' + @property + def _compare_file_extension(self): + """The default extension to use when outputting check files in COMPARE_CHECK mode.""" + if self.stderr_output: + return 'txt' + return self.compare_file_extension + def test_all_comparisons(self): """Testing all comparisons""" if not isinstance(self.compare_file, (list, tuple)): @@ -300,50 +320,86 @@ class ComparisonMixin: for args in self.comparisons: self.assertCompare( compare_file, - self.get_compare_outfile(args, addout), + self.get_compare_cmpfile(args, addout), args, ) - def assertCompare(self, infile, outfile, args): #pylint: disable=invalid-name + def assertCompare(self, infile, cmpfile, args, outfile=None): #pylint: disable=invalid-name """ Compare the output of a previous run against this one. - infile: The filename of the pre-processed svg (or other type of file) - - outfile: The filename of the data we expect to get, if not set + - cmpfile: The filename of the data we expect to get, if not set the filename will be generated from the effect name and kwargs. - args: All the arguments to be passed to the effect run + - outfile: Optional, instead of returning a regular output, this extension + dumps it's output to this filename instead. """ + compare_mode = int(os.environ.get('EXPORT_COMPARE', COMPARE_DELETE)) + effect = self.assertEffect(infile, args=args) - if outfile is None: - outfile = self.get_compare_outfile(args) + if cmpfile is None: + cmpfile = self.get_compare_cmpfile(args) - if not os.path.isfile(outfile): - raise IOError(f"Comparison file {outfile} not found") + if not os.path.isfile(cmpfile) and compare_mode == COMPARE_DELETE: + raise IOError( + f"Comparison file {cmpfile} not found, set EXPORT_COMPARE=1 to create it.") - data_a = effect.test_output.getvalue() - if os.environ.get('EXPORT_COMPARE', False): - with open(outfile + '.export', 'wb') as fhl: - if sys.version_info[0] == 3 and isinstance(data_a, str): + if outfile: + if not os.path.isabs(outfile): + outfile = os.path.join(self.tempdir, outfile) + self.assertTrue(os.path.isfile(outfile), "No output file created! {}".format(outfile)) + with open(outfile, 'rb') as fhl: + data_a = fhl.read() + else: + data_a = effect.test_output.getvalue() + + write_output = None + if compare_mode == COMPARE_CHECK: + _file = cmpfile[:-4] if cmpfile.endswith('.out') else cmpfile + write_output = f"{_file}.{self._compare_file_extension}" + elif (compare_mode == COMPARE_WRITE and not os.path.isfile(cmpfile))\ + or compare_mode == COMPARE_OVERWRITE: + write_output = cmpfile + + try: + if write_output and not os.path.isfile(cmpfile): + raise AssertionError(f"Check the output: {write_output}") + with open(cmpfile, 'rb') as fhl: + data_b = self._apply_compare_filters(fhl.read(), False) + self._base_compare(data_a, data_b, compare_mode) + except AssertionError: + if write_output: + if isinstance(data_a, str): data_a = data_a.encode('utf-8') - fhl.write(self._apply_compare_filters(data_a, True)) - print(f"Written output: {outfile}.export") - + with open(write_output, 'wb') as fhl: + fhl.write(self._apply_compare_filters(data_a, True)) + print(f"Written output: {write_output}") + # This only reruns if the original test failed. + # The idea here is to make sure the new output file is "stable" + # Because some tests can produce random changes and we don't + # want test authors to be too reassured by a simple write. + if write_output == cmpfile: + effect = self.assertEffect(infile, args=args) + self._base_compare(data_a, cmpfile) + if not write_output == cmpfile: + raise + + def _base_compare(self, data_a, data_b, compare_mode): data_a = self._apply_compare_filters(data_a) - with open(outfile, 'rb') as fhl: - data_b = self._apply_compare_filters(fhl.read(), False) - if isinstance(data_a, bytes) and isinstance(data_b, bytes) \ and data_a.startswith(b'<') and data_b.startswith(b'<'): # Late importing diff_xml, delta = xmldiff(data_a, data_b) - if not delta and not os.environ.get('EXPORT_COMPARE', False): - print('The XML is different, you can save the output using the EXPORT_COMPARE=1'\ - ' envionment variable. This will save the compared file as a ".output" file'\ - ' next to the reference file used in the test.\n') - diff = f"SVG Differences: {outfile}\n\n" + if not delta and compare_mode == COMPARE_DELETE: + print('The XML is different, you can save the output using the EXPORT_COMPARE'\ + ' envionment variable. Set it to 1 to save a file you can check, set it to'\ + ' 3 to overwrite this comparison, setting the new data as the correct one.\n' + ) + diff = f"SVG Differences\n\n" if os.environ.get('XML_DIFF', False): diff = '<- ' + diff_xml else: @@ -368,7 +424,7 @@ class ComparisonMixin: data = cfilter(data) return data - def get_compare_outfile(self, args, addout=None): + def get_compare_cmpfile(self, args, addout=None): """Generate an output file for the arguments given""" if addout is not None: args = list(args) + [str(addout)] @@ -381,4 +437,4 @@ class ComparisonMixin: # avoid filename-too-long error opstr = hashlib.md5(opstr.encode('latin1')).hexdigest() opstr = '__' + opstr - return self.data_file("refs", f"{self.effect_name}{opstr}.out") + return self.data_file("refs", f"{self.effect_name}{opstr}.out", check_exists=False) diff --git a/tests/data/refs/image_extract__--selectedonly__False__--filepath__TMP_DIR__img__.out b/tests/data/refs/image_extract__--selectedonly__False__--filepath__TMP_DIR__.out similarity index 100% rename from tests/data/refs/image_extract__--selectedonly__False__--filepath__TMP_DIR__img__.out rename to tests/data/refs/image_extract__--selectedonly__False__--filepath__TMP_DIR__.out diff --git a/tests/data/refs/image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__img__.out b/tests/data/refs/image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__.out similarity index 100% rename from tests/data/refs/image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__img__.out rename to tests/data/refs/image_extract__--selectedonly__True__--id__embeded_image01__--filepath__TMP_DIR__.out diff --git a/tests/test_gcodetools.py b/tests/test_gcodetools.py index f98fbb09c..cdf05d02a 100644 --- a/tests/test_gcodetools.py +++ b/tests/test_gcodetools.py @@ -36,8 +36,10 @@ class TestGcodetoolsBasic(ComparisonMixin, InkscapeExtensionTestMixin, TestCase) FILESET + ('--active-tab="tools_library"',), FILESET + ('--active-tab="lathe_modify_path"',), FILESET + ('--active-tab="offset"',), + FILESET + ('--active-tab="plasma-prepare-path"',), ] compare_filters = [CompareOrderIndependentBytes()] + compare_file_extension = 'dxf' def test_all_comparisons(self): """ @@ -54,30 +56,5 @@ class TestGcodetoolsBasic(ComparisonMixin, InkscapeExtensionTestMixin, TestCase) '--directory={}'.format(self.tempdir), '--filename=output.ngc', ) - self.assertEffect(self.compare_file, args=args) - outfile = os.path.join(self.tempdir, 'output.ngc') - self.assertTrue(os.path.isfile(outfile), "No output file created! {}".format(outfile)) - - with open(outfile, 'rb') as fhl: - data_a = fhl.read() - - self.assertTrue(data_a, "No data produced with {}".format(tab)) - - outfile = self.get_compare_outfile(args) - if os.environ.get('EXPORT_COMPARE', False): - with open(outfile + '.export', 'wb') as fhl: - fhl.write(data_a) - print("Written output: {}.export".format(outfile)) - - with open(outfile, 'rb') as fhl: - data_b = fhl.read() - - self.assertEqual(data_a, data_b) - -if sys.version_info[0] == 3: - # This changes output between python2 and python3, we don't know - # why and don't have the gcodetool developers to help us understand. - TestGcodetoolsBasic.comparisons.append( - FILESET + ('--active-tab="plasma-prepare-path"',), - ) + self.assertCompare(self.compare_file, None, args, 'output.ngc') diff --git a/tests/test_guillotine.py b/tests/test_guillotine.py index d478491cc..4af8e04f9 100644 --- a/tests/test_guillotine.py +++ b/tests/test_guillotine.py @@ -30,7 +30,7 @@ class TestGuillotineBasic(ComparisonMixin, TestCase): self.assertEffect(compare_file, args=args) self.assertTrue(os.path.isdir(outdir)) - infile = self.get_compare_outfile(args) + infile = self.get_compare_cmpfile(args) if os.environ.get('EXPORT_COMPARE', False): self.export_comparison(outdir, infile) @@ -41,9 +41,9 @@ class TestGuillotineBasic(ComparisonMixin, TestCase): self.assertEqual(fileobj.read(), fhl.read(), "File '{}'".format(item.name)) @staticmethod - def export_comparison(outdir, outfile): + def export_comparison(outdir, cmpfile): """Export the files as a tar file for manual comparison""" - tarname = outfile + '.export' + tarname = cmpfile + '.export' tar = tarfile.open(tarname, 'w|') # We make a tar archive so we can test it. @@ -55,4 +55,4 @@ class TestGuillotineBasic(ComparisonMixin, TestCase): fhl.seek(0) tar.addfile(info, fhl) tar.close() - print("Written output: {}.export".format(outfile)) + print("Written output: {}.export".format(cmpfile)) diff --git a/tests/test_hershey.py b/tests/test_hershey.py index f76eac8f1..123a30d3b 100644 --- a/tests/test_hershey.py +++ b/tests/test_hershey.py @@ -9,21 +9,21 @@ from inkex.tester.svg import svg, svg_file, uu_svg from hershey import Hershey class HersheyComparisonMixin(ComparisonMixin): - comparisons_outfile_dict = {} # pairs of args and expected outputs + comparisons_cmpfile_dict = {} # pairs of args and expected outputs def setUp(self): self.effect_class = Hershey self.compare_filters = [CompareNumericFuzzy(), CompareOrderIndependentStyle()] - self.comparisons = self.comparisons_outfile_dict.keys() + self.comparisons = self.comparisons_cmpfile_dict.keys() - def get_compare_outfile(self, args, addout=None): - ''' get the correct outfile to compare from comparisons_dict; ''' - return self.data_file('refs', self.comparisons_outfile_dict[args]) + def get_compare_cmpfile(self, args, addout=None): + ''' get the correct cmpfile to compare from comparisons_dict; ''' + return self.data_file('refs', self.comparisons_cmpfile_dict[args]) class TestHersheyBasic(InkscapeExtensionTestMixin, HersheyComparisonMixin, TestCase): compare_file = 'svg/hershey_input.svg' # a huge number of inputs - comparisons_outfile_dict = { + comparisons_cmpfile_dict = { # default parameters: (): 'hershey.out', # same as above, but explicit parameters. same output: @@ -32,7 +32,7 @@ class TestHersheyBasic(InkscapeExtensionTestMixin, HersheyComparisonMixin, TestC class TestHersheyTrivialInput(InkscapeExtensionTestMixin, HersheyComparisonMixin, TestCase): compare_file = 'svg/hershey_trivial_input.svg' - comparisons_outfile_dict = { + comparisons_cmpfile_dict = { # loading a different font: ('--fontface="EMSAllure"', ): 'hershey_loadfont.out', # using the "other font" option. same output as above: @@ -45,7 +45,7 @@ class TestHersheyTrivialInput(InkscapeExtensionTestMixin, HersheyComparisonMixin class TestHersheyTables(InkscapeExtensionTestMixin, HersheyComparisonMixin, TestCase): compare_file = 'svg/default-inkscape-SVG.svg' - comparisons_outfile_dict = { + comparisons_cmpfile_dict = { # generates a simple font table: ('--tab="utilities"', '--action="sample"', '--text="I am a quick brown fox"'): 'hershey_fonttable.out', # generates a simple font table, while testing UTF-8 input diff --git a/tests/test_image_extract.py b/tests/test_image_extract.py index fa7fe096b..0a1e20e2f 100644 --- a/tests/test_image_extract.py +++ b/tests/test_image_extract.py @@ -8,6 +8,7 @@ class ExtractImageBasicTest(ComparisonMixin, InkscapeExtensionTestMixin, TestCas stderr_protect = False effect_class = ExtractImage compare_file = 'svg/images.svg' + compare_file_extension = 'png' comparisons = [ ('--selectedonly=False',), ('--selectedonly=True', '--id=embeded_image01'), @@ -16,25 +17,5 @@ class ExtractImageBasicTest(ComparisonMixin, InkscapeExtensionTestMixin, TestCas def test_all_comparisons(self): """Images are extracted to a file directory""" for args in self.comparisons: - outdir = os.path.join(self.tempdir, 'img') - args += ('--filepath={}/'.format(outdir),) - self.assertEffect(self.compare_file, args=args) - - outfile = os.path.join(outdir, 'embeded_image01.png') - self.assertTrue(os.path.isfile(outfile), "No output file created! {}".format(outfile)) - - with open(outfile, 'rb') as fhl: - data_a = fhl.read() - - self.assertTrue(data_a, "No data produced with {}".format(args)) - - outfile = self.get_compare_outfile(args) - if os.environ.get('EXPORT_COMPARE', False): - with open(outfile + '.export', 'wb') as fhl: - fhl.write(data_a) - print("Written output: {}.export".format(outfile)) - - with open(outfile, 'rb') as fhl: - data_b = fhl.read() - - self.assertEqual(data_a, data_b) + args += ('--filepath={}/'.format(self.tempdir),) + self.assertCompare(self.compare_file, None, args, 'embeded_image01.png') -- GitLab