From ee4a7d331664cb2e40a249a58cc9fa1cf3ed0702 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Fri, 1 Jan 2021 19:23:51 -0800 Subject: [PATCH] use f-strings in Python test scripts Now that we have Python 3.6 as a base line requirement, we can use f-strings pervasively. Related to #1870. --- ci/tests.py | 18 ++-- rtest/rtest.py | 89 +++++++++---------- rtest/test_examples.py | 6 +- rtest/test_regression.py | 12 +-- rtest/test_tools.py | 6 +- .../installation/test_installation.py | 2 +- .../regression_test_helpers.py | 8 +- tests/regression_tests/shapes/shapes.py | 10 +-- tests/regression_tests/shapes/test_shapes.py | 6 +- tests/regression_tests/vuln/vuln.py | 10 +-- 10 files changed, 78 insertions(+), 89 deletions(-) diff --git a/ci/tests.py b/ci/tests.py index 35a6fe543..4098ec33a 100644 --- a/ci/tests.py +++ b/ci/tests.py @@ -104,13 +104,13 @@ def test_existence(binary: str): # https://gitlab.com/graphviz/graphviz/-/issues/1839 is fixed if os_id in ['centos', 'fedora'] and binary == 'dot_builtins': check_that_tool_does_not_exist(binary, os_id) - pytest.skip('dot_builtins is not installed for ' + os_id + ' (#1839)') + pytest.skip(f'dot_builtins is not installed for {os_id} (#1839)') # FIXME: Remove skip when # https://gitlab.com/graphviz/graphviz/-/issues/1835 is fixed if os_id in ['ubuntu', 'centos'] and binary == 'mingle': check_that_tool_does_not_exist(binary, os_id) - pytest.skip('mingle is not built for ' + os_id + ' (#1835)') + pytest.skip(f'mingle is not built for {os_id} (#1835)') # FIXME: Remove skip when # https://gitlab.com/graphviz/graphviz/-/issues/1834 is fixed @@ -124,14 +124,14 @@ def test_existence(binary: str): if os.getenv('build_system') == 'cmake': if binary in tools_not_built_with_cmake: check_that_tool_does_not_exist(binary, os_id) - pytest.skip(binary + ' is not built with CMake (#1753 & #1836)') + pytest.skip(f'{binary} is not built with CMake (#1753 & #1836)') # FIXME: Remove skip when # https://gitlab.com/graphviz/graphviz/-/issues/1837 is fixed if os.getenv('build_system') == 'msbuild': if binary in tools_not_built_with_msbuild: check_that_tool_does_not_exist(binary, os_id) - pytest.skip(binary + ' is not built with MSBuild (#1837)') + pytest.skip(f'{binary} is not built with MSBuild (#1837)') # FIXME: Remove skip when # https://gitlab.com/graphviz/graphviz/-/issues/1854 is fixed @@ -139,14 +139,10 @@ def test_existence(binary: str): if platform.system() == 'Darwin': if binary in tools_not_built_with_autotools_on_macos: check_that_tool_does_not_exist(binary, os_id) - pytest.skip(binary + ' is not built with autotools on macOS (#1854)') + pytest.skip(f'{binary} is not built with autotools on macOS (#1854)') assert shutil.which(binary) is not None def check_that_tool_does_not_exist(tool, os_id): - assert shutil.which(tool) is None, '{} has been resurrected in the {} ' \ - 'build on {}. Please remove skip.'.format( - tool, - os.getenv('build_system'), - os_id - ) + assert shutil.which(tool) is None, f'{tool} has been resurrected in the ' \ + f'{os.getenv("build_system")} build on {os_id}. Please remove skip.' diff --git a/rtest/rtest.py b/rtest/rtest.py index 2af10e1e4..9def06831 100755 --- a/rtest/rtest.py +++ b/rtest/rtest.py @@ -36,9 +36,9 @@ CRASH_CNT = 0 DIFF_CNT = 0 TOT_CNT = 0 TESTTYPES = {} -TMPINFILE = 'tmp{}.gv'.format(os.getpid()) -TMPFILE1 = 'tmpnew{}'.format(os.getpid()) -TMPFILE2 = 'tmpref{}'.format(os.getpid()) +TMPINFILE = f'tmp{os.getpid()}.gv' +TMPFILE1 = f'tmpnew{os.getpid()}' +TMPFILE2 = f'tmpref{os.getpid()}' # Read single line, storing it in LINE. # Returns the line on success, else returns None @@ -148,36 +148,35 @@ def doDiff(OUTFILE, OUTDIR, REFDIR, testname, subtest_index, fmt): elif F == 'png': # FIXME: remove when https://gitlab.com/graphviz/graphviz/-/issues/1788 is fixed if os.environ.get('build_system') == 'cmake': - print('Warning: Skipping PNG image comparison for test {0}:{1} : format: ' - '{2} because CMake builds does not contain the diffimg ' - 'utility (#1788)' - .format(testname, subtest_index, fmt), + print(f'Warning: Skipping PNG image comparison for test {testname}:' + f'{subtest_index} : format: {fmt} because CMake builds does not ' + 'contain the diffimg utility (#1788)', file=sys.stderr) return returncode = subprocess.call( - [DIFFIMG, FILE1, FILE2, os.path.join(OUTHTML, 'dif_' + OUTFILE)], + [DIFFIMG, FILE1, FILE2, os.path.join(OUTHTML, f'dif_{OUTFILE}')], ) if returncode != 0: with open(os.path.join(OUTHTML, 'index.html'), mode='a') as fd: fd.write('

\n') - shutil.copyfile(FILE2, os.path.join(OUTHTML, 'old_' + OUTFILE)) - fd.write('\n'.format(OUTFILE)) - shutil.copyfile(FILE1, os.path.join(OUTHTML, 'new_' + OUTFILE)) - fd.write('\n'.format(OUTFILE)) - fd.write('\n'.format(OUTFILE)) + shutil.copyfile(FILE2, os.path.join(OUTHTML, f'old_{OUTFILE}')) + fd.write(f'\n') + shutil.copyfile(FILE1, os.path.join(OUTHTML, f'new_{OUTFILE}')) + fd.write(f'\n') + fd.write(f'\n') else: - os.unlink(os.path.join(OUTHTML, 'dif_' + OUTFILE)) + os.unlink(os.path.join(OUTHTML, f'dif_{OUTFILE}')) else: returncode = subprocess.call( ['diff', '--strip-trailing-cr', FILE2, FILE1], stdout=subprocess.DEVNULL, ) if returncode != 0: - print('Test {0}:{1} : == Failed == {2}'.format(testname, subtest_index, OUTFILE), file=sys.stderr) + print(f'Test {testname}:{subtest_index} : == Failed == {OUTFILE}', file=sys.stderr) DIFF_CNT += 1 else: if VERBOSE: - print('Test {0}:{1} : == OK == {2}'.format(testname, subtest_index, OUTFILE), file=sys.stderr) + print(f'Test {testname}:{subtest_index} : == OK == {OUTFILE}', file=sys.stderr) # Generate output file name given 3 parameters. # testname layout format @@ -189,7 +188,7 @@ def genOutname(name, alg, fmt): fmt_split = fmt.split(':') if len(fmt_split) >= 2: F = fmt_split[0] - XFMT = '_' + '_'.join(fmt_split[1:]) + XFMT = f'_{"_".join(fmt_split[1:])}' else: F=fmt XFMT='' @@ -202,7 +201,7 @@ def genOutname(name, alg, fmt): else: TESTTYPES[IDX]= j + 1 J = str(j) - OUTFILE = name + '_' + alg + XFMT + J + '.' + F + OUTFILE = f'{name}_{alg}{XFMT}{J}.{F}' return OUTFILE def doTest(TEST): @@ -215,7 +214,7 @@ def doTest(TEST): return GRAPH = TEST['GRAPH'] if GRAPH == '=': - INFILE = os.path.join(GRAPHDIR, TESTNAME + '.gv') + INFILE = os.path.join(GRAPHDIR, f'{TESTNAME}.gv') elif GRAPH.startswith('graph') or GRAPH.startswith('digraph'): with open(TMPINFILE, mode='w') as fd: fd.write(GRAPH) @@ -223,7 +222,7 @@ def doTest(TEST): elif os.path.splitext(GRAPH)[1] == '.gv': INFILE = os.path.join(GRAPHDIR, GRAPH) else: - print('Unknown graph spec, test {} - ignoring'.format(TESTNAME), + print(f'Unknown graph spec, test {TESTNAME} - ignoring', file=sys.stderr) return @@ -233,8 +232,8 @@ def doTest(TEST): OUTPATH = os.path.join(OUTDIR, OUTFILE) KFLAGS = SUBTEST['ALG'] TFLAGS = SUBTEST['FMT'] - if KFLAGS: KFLAGS = '-K{}'.format(KFLAGS) - if TFLAGS: TFLAGS = '-T{}'.format(TFLAGS) + if KFLAGS: KFLAGS = f'-K{KFLAGS}' + if TFLAGS: TFLAGS = f'-T{TFLAGS}' testcmd = [DOT] if KFLAGS: testcmd += [KFLAGS] if TFLAGS: testcmd += [TFLAGS] @@ -247,9 +246,8 @@ def doTest(TEST): # fixed if os.environ.get('build_system') == 'cmake' and \ SUBTEST['FMT'] == 'png:gd': - print('Skipping test {0}:{1} : format {2} because CMake builds ' - 'does not support format png:gd (#1786)' - .format(TESTNAME, i, SUBTEST['FMT']), + print(f'Skipping test {TESTNAME}:{i} : format {SUBTEST["FMT"]} because ' + 'CMake builds does not support format png:gd (#1786)', file=sys.stderr) continue # FIXME: Remove when https://gitlab.com/graphviz/graphviz/-/issues/1269 is @@ -257,10 +255,9 @@ def doTest(TEST): if platform.system() == 'Windows' and \ os.environ.get('build_system') == 'msbuild' and \ '-Goverlap=false' in SUBTEST['FLAGS']: - print('Skipping test {0}:{1} : with flag -Goverlap=false because it fails ' - 'with Windows MSBuild builds which are not built with ' - 'triangulation library (#1269)' - .format(TESTNAME, i), + print(f'Skipping test {TESTNAME}:{i} : with flag -Goverlap=false because ' + 'it fails with Windows MSBuild builds which are not built with ' + 'triangulation library (#1269)', file=sys.stderr) continue # FIXME: Remove when https://gitlab.com/graphviz/graphviz/-/issues/1787 is @@ -269,18 +266,16 @@ def doTest(TEST): os.environ.get('build_system') == 'msbuild' and \ os.environ.get('configuration') == 'Debug' and \ TESTNAME == 'user_shapes': - print('Skipping test {0}:{1} : using shapefile because it fails ' - 'with Windows MSBuild Debug builds (#1787)' - .format(TESTNAME, i), + print(f'Skipping test {TESTNAME}:{i} : using shapefile because it fails ' + 'with Windows MSBuild Debug builds (#1787)', file=sys.stderr) continue # FIXME: Remove when https://gitlab.com/graphviz/graphviz/-/issues/1790 is # fixed if platform.system() == 'Windows' and \ TESTNAME == 'ps_user_shapes': - print('Skipping test {0}:{1} : using PostScript shapefile because it ' - 'fails with Windows builds (#1790)' - .format(TESTNAME, i), + print(f'Skipping test {TESTNAME}:{i} : using PostScript shapefile ' + 'because it fails with Windows builds (#1790)', file=sys.stderr) continue @@ -297,14 +292,14 @@ def doTest(TEST): if RVAL != 0 or not os.path.exists(OUTPATH): CRASH_CNT += 1 - print('Test {0}:{1} : == Layout failed =='.format(TESTNAME, i), file=sys.stderr) - print(' ' + ' '.join(testcmd), file=sys.stderr) + print(f'Test {TESTNAME}:{i} : == Layout failed ==', file=sys.stderr) + print(f' {" ".join(testcmd)}', file=sys.stderr) elif GENERATE: continue elif os.path.exists(os.path.join(REFDIR, OUTFILE)): doDiff(OUTFILE, OUTDIR, REFDIR, TESTNAME, i, SUBTEST['FMT']) else: - print('Test {0}:{1} : == No file {2}/{3} for comparison =='.format(TESTNAME, i, REFDIR, OUTFILE), file=sys.stderr) + print(f'Test {TESTNAME}:{i} : == No file {REFDIR}/{OUTFILE} for comparison ==', file=sys.stderr) # clear TESTTYPES TESTTYPES = {} @@ -361,14 +356,14 @@ if args.testfile: if os.path.exists(args.testfile): TESTFILE = args.testfile else: - print('Test file {0} does not exist'.format(args.testfile), file=sys.stderr) + print(f'Test file {args.testfile} does not exist', file=sys.stderr) sys.exit(1) # Check environment and initialize if not NOOP: if not os.path.isdir(REFDIR): - print('Test data directory {0} does not exist'.format(REFDIR), + print(f'Test data directory {REFDIR} does not exist', file=sys.stderr) sys.exit(1) @@ -384,13 +379,13 @@ if not DOT: print('Could not find a value for DOT', file=sys.stderr) sys.exit(1) if not os.path.isfile(DOT) or not os.access(DOT, os.X_OK): - print('{0} program is not executable'.format(DOT)) + print(f'{DOT} program is not executable') sys.exit(1) if not GENERATE: if DIFFIMG: if not os.path.isfile(DIFFIMG) or not os.access(DIFFIMG, os.X_OK): - print('{0} program is not executable'.format(DIFFIMG)) + print(f'{DIFFIMG} program is not executable') sys.exit(1) else: print('Could not find a value for DIFFIMG', file=sys.stderr) @@ -408,13 +403,11 @@ while True: break doTest(TEST) if NOOP: - print('No. tests: ' + str(TOT_CNT), file=sys.stderr) + print(f'No. tests: {TOT_CNT}', file=sys.stderr) elif GENERATE: - print('No. tests: ' + str(TOT_CNT) + ' Layout failures: ' + str(CRASH_CNT), file=sys.stderr) + print(f'No. tests: {TOT_CNT} Layout failures: {CRASH_CNT}', file=sys.stderr) else: - print('No. tests: ' + - str(TOT_CNT) + ' Layout failures: ' + - str(CRASH_CNT) + ' Changes: ' + - str(DIFF_CNT), file=sys.stderr) + print(f'No. tests: {TOT_CNT} Layout failures: {CRASH_CNT} Changes: ' + f'{DIFF_CNT}', file=sys.stderr) EXIT_STATUS = CRASH_CNT + DIFF_CNT sys.exit(EXIT_STATUS) diff --git a/rtest/test_examples.py b/rtest/test_examples.py index face5f6e5..3c8433408 100644 --- a/rtest/test_examples.py +++ b/rtest/test_examples.py @@ -42,10 +42,10 @@ def test_compile_example(src): debug = os.environ.get('configuration') == 'Debug' rt_lib_option = '-MDd' if debug else '-MD' subprocess.check_call([cc, filepath, '-Fe:', exe, '-nologo', - rt_lib_option, '-link'] + ['{}.lib'.format(l) for l in libs]) + rt_lib_option, '-link'] + [f'{l}.lib' for l in libs]) else: subprocess.check_call([cc, '-std=c99', '-o', exe, filepath] - + ['-l{}'.format(l) for l in libs]) + + [f'-l{l}' for l in libs]) # FIXME: Remove skip of execution of neatopack.c example when # https://gitlab.com/graphviz/graphviz/-/issues/1800 has been fixed @@ -61,7 +61,7 @@ def test_compile_example(src): universal_newlines=True, ) p.communicate(input='graph {a -- b}') - print('returncode: {} = 0x{:08x}'.format(p.returncode, p.returncode)) + print(f'returncode: {p.returncode} = 0x{p.returncode:08x}') assert p.returncode == 0 @pytest.mark.parametrize('src', ['addrings', 'attr', 'bbox', 'bipart', diff --git a/rtest/test_regression.py b/rtest/test_regression.py index e1526eab1..8f920259a 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -552,7 +552,7 @@ def test_1869(variant: int): ''' # locate our associated test case in this directory - input = Path(__file__).parent / '1869-{}.gml'.format(variant) + input = Path(__file__).parent / f'1869-{variant}.gml' assert input.exists(), 'unexpectedly missing test case' # ask gml2gv to translate it to DOT @@ -692,7 +692,7 @@ def test_1913(): assert stderr.strip() == '' # these attributes should also be valid when title cased - input = '{}{}'.format(align[0].upper(), align[1:]) + input = f'{align[0].upper()}{align[1:]}' ret, stderr = run(graph.format(input)) assert ret == 0 assert stderr.strip() == '' @@ -709,17 +709,17 @@ def test_1913(): input = align _, stderr = run(graph.format(input)) - assert 'Warning: Illegal value {} for ALIGN - ignored'.format(input) in stderr + assert f'Warning: Illegal value {input} for ALIGN - ignored' in stderr # these attributes should also fail when title cased - input = '{}{}'.format(align[0].upper(), align[1:]) + input = f'{align[0].upper()}{align[1:]}' _, stderr = run(graph.format(input)) - assert 'Warning: Illegal value {} for ALIGN - ignored'.format(input) in stderr + assert f'Warning: Illegal value {input} for ALIGN - ignored' in stderr # similarly, they should fail when upper cased input = align.upper() _, stderr = run(graph.format(input)) - assert 'Warning: Illegal value {} for ALIGN - ignored'.format(input) in stderr + assert f'Warning: Illegal value {input} for ALIGN - ignored' in stderr def test_1931(): ''' diff --git a/rtest/test_tools.py b/rtest/test_tools.py index 428ac75be..6b92d5b57 100644 --- a/rtest/test_tools.py +++ b/rtest/test_tools.py @@ -54,7 +54,7 @@ import shutil def test_tools(tool): if shutil.which(tool) is None: - pytest.skip('{} not available'.format(tool)) + pytest.skip(f'{tool} not available') # FIXME: Remove skip when # https://gitlab.com/graphviz/graphviz/-/issues/1829 is fixed @@ -81,7 +81,7 @@ def test_tools(tool): universal_newlines=True, ) assert re.match('usage', output, flags=re.IGNORECASE) is not None, \ - tool +' -? did not show usage' + f'{tool} -? did not show usage' # Test unsupported option returncode = subprocess.call( @@ -89,4 +89,4 @@ def test_tools(tool): env=environ_copy, ) - assert returncode != 0, tool + ' accepted unsupported option -$' + assert returncode != 0, f'{tool} accepted unsupported option -$' diff --git a/tests/regression_tests/installation/test_installation.py b/tests/regression_tests/installation/test_installation.py index 5ac27c203..ec5a2a399 100644 --- a/tests/regression_tests/installation/test_installation.py +++ b/tests/regression_tests/installation/test_installation.py @@ -25,5 +25,5 @@ def test_installation(): try: actual_version = actual_version_string.split()[4] except IndexError: - pytest.fail('Malformed version string: {0}'.format(actual_version_string)) + pytest.fail(f'Malformed version string: {actual_version_string}') assert actual_version == expected_version diff --git a/tests/regression_tests/regression_test_helpers.py b/tests/regression_tests/regression_test_helpers.py index 54cfa87cb..2a19c0aa0 100644 --- a/tests/regression_tests/regression_test_helpers.py +++ b/tests/regression_tests/regression_test_helpers.py @@ -2,11 +2,11 @@ import difflib from pathlib import Path def compare_graphs(name, output_type): - filename = Path(name + '.' + output_type) + filename = Path(f'{name}.{output_type}') filename_reference = Path('reference') / filename filename_output = Path('output') / filename if not filename_reference.is_file(): - print('Failure: ' + str(filename) + ' - No reference file present.') + print(f'Failure: {filename} - No reference file present.') return False with open(filename_reference) as reference_file: @@ -21,7 +21,7 @@ def compare_graphs(name, output_type): diff.append(line) if len(diff) == 0: - print('Success: ' + str(filename)) + print(f'Success: {filename}') return True else: difference = Path('difference') @@ -36,5 +36,5 @@ def compare_graphs(name, output_type): with open('difference/' + str(filename), 'w') as diff_file: diff_file.writelines(diff) - print('Failure: ' + str(filename) + ' - Generated file does not match reference file.') + print(f'Failure: {filename} - Generated file does not match reference file.') return False diff --git a/tests/regression_tests/shapes/shapes.py b/tests/regression_tests/shapes/shapes.py index ea73c8070..8c8f3affe 100644 --- a/tests/regression_tests/shapes/shapes.py +++ b/tests/regression_tests/shapes/shapes.py @@ -81,14 +81,14 @@ def generate_shape_graph(shape, output_type): if not Path('output').exists(): Path('output').mkdir(parents=True) - output_file = Path('output') / (shape + '.' + output_type) + output_file = Path('output') / f'{shape}.{output_type}' process = Popen(['dot', '-T' + output_type, '-o', output_file], stdin=PIPE) - input_graph = 'graph G { a [label="" shape=' + shape + '] }' + input_graph = f'graph G {{ a [label="" shape={shape}] }}' process.communicate(input = input_graph.encode('utf_8')) if process.wait() != 0: - print('An error occurred while generating: ' + str(output_file)) + print(f'An error occurred while generating: {output_file}') exit(1) if output_type == 'svg': @@ -115,8 +115,8 @@ for shape in shapes: print('') print('Results for "shapes" regression test:') -print(' Number of tests: ' + str(len(shapes) * len(output_types))) -print(' Number of failures: ' + str(failures)) +print(f' Number of tests: {len(shapes) * len(output_types)}') +print(f' Number of failures: {failures}') if not failures == 0: exit(1) diff --git a/tests/regression_tests/shapes/test_shapes.py b/tests/regression_tests/shapes/test_shapes.py index 188bb65d8..569d64315 100644 --- a/tests/regression_tests/shapes/test_shapes.py +++ b/tests/regression_tests/shapes/test_shapes.py @@ -83,14 +83,14 @@ def generate_shape_graph(shape, output_type): if not Path('output').exists(): Path('output').mkdir(parents=True) - output_file = Path('output') / (shape + '.' + output_type) + output_file = Path('output') / f'{shape}.{output_type}' process = Popen(['dot', '-T' + output_type, '-o', output_file], stdin=PIPE) - input_graph = 'graph G { a [label="" shape=' + shape + '] }' + input_graph = f'graph G {{ a [label="" shape={shape}] }}' process.communicate(input = input_graph.encode('utf_8')) if process.wait() != 0: - print('An error occurred while generating: ' + str(output_file)) + print(f'An error occurred while generating: {output_file}') exit(1) if output_type == 'svg': diff --git a/tests/regression_tests/vuln/vuln.py b/tests/regression_tests/vuln/vuln.py index 0afa153f3..7b6fa498e 100644 --- a/tests/regression_tests/vuln/vuln.py +++ b/tests/regression_tests/vuln/vuln.py @@ -18,12 +18,12 @@ def generate_vuln_graph(vulnfile, output_type): if not Path('output').exists(): Path('output').mkdir(parents=True) - output_file = Path('output') / (vulnfile + '.' + output_type[0]) - input_file = Path('input') / (vulnfile + '.dot') + output_file = Path('output') / f'{vulnfile}.{output_type[0]}' + input_file = Path('input') / f'{vulnfile}.dot' process = Popen(['dot', '-T' + output_type[1], '-o', output_file, input_file], stdin=PIPE) if process.wait() < 0: - print('An error occurred while generating: ' + str(output_file)) + print(f'An error occurred while generating: {output_file}') exit(1) failures = 0 @@ -35,8 +35,8 @@ for vulnfile in vulnfiles: print('') print('Results for "vuln" regression test:') -print(' Number of tests: ' + str(len(vulnfiles) * len(output_types))) -print(' Number of failures: ' + str(failures)) +print(f' Number of tests: {len(vulnfiles) * len(output_types)}') +print(f' Number of failures: {failures}') if not failures == 0: exit(1) -- 2.40.0