From 887f4a1b06ceed33ee6ba4c5589a32a607d6b001 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Thu, 14 Feb 2019 12:42:00 -0700 Subject: Clean up some tests: use context managers Plenty of complaints coming from Python 3.8alpha on unclosed files. Targeted those areas which intersect with PyPy failures - this changeset reduces the PyPy fails by 17 on the local test environment. So this affects both Issue #3299 and the PyPy support project. Signed-off-by: Mats Wichmann --- src/engine/SCons/ActionTests.py | 25 +++++++++------- src/engine/SCons/BuilderTests.py | 14 +++++---- src/engine/SCons/SConfTests.py | 6 ++-- src/engine/SCons/TaskmasterTests.py | 1 + src/engine/SCons/cppTests.py | 3 +- test/Batch/generated.py | 10 ++++--- test/CacheDir/NoCache.py | 3 +- test/Deprecated/TargetSignatures/build-content.py | 3 +- test/Fortran/link-with-cxx.py | 6 ++-- test/MSVC/generate-rc.py | 8 ++--- test/NodeOps.py | 3 +- test/Repository/Local.py | 6 ++-- test/Repository/option-c.py | 3 +- test/Requires/eval-order.py | 10 ++++--- test/Value.py | 10 +++++-- test/VariantDir/File-create.py | 6 ++-- test/chained-build.py | 6 ++-- test/implicit-cache/basic.py | 3 +- test/sconsign/nonwritable.py | 12 +++++--- testing/framework/TestSCons.py | 3 +- testing/framework/TestSCons_time.py | 36 +++++++++++++++++++---- 21 files changed, 119 insertions(+), 58 deletions(-) diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py index 3e83b50..efe6e98 100644 --- a/src/engine/SCons/ActionTests.py +++ b/src/engine/SCons/ActionTests.py @@ -62,25 +62,28 @@ test = TestCmd.TestCmd(workdir='') test.write('act.py', """\ import os, string, sys -f = open(sys.argv[1], 'w') -f.write("act.py: '" + "' '".join(sys.argv[2:]) + "'\\n") -try: - if sys.argv[3]: - f.write("act.py: '" + os.environ[sys.argv[3]] + "'\\n") -except: - pass -f.close() + +with open(sys.argv[1], 'w') as f: + f.write("act.py: '" + "' '".join(sys.argv[2:]) + "'\\n") + try: + if sys.argv[3]: + f.write("act.py: '" + os.environ[sys.argv[3]] + "'\\n") + except: + pass + if 'ACTPY_PIPE' in os.environ: if 'PIPE_STDOUT_FILE' in os.environ: - stdout_msg = open(os.environ['PIPE_STDOUT_FILE'], 'r').read() + with open(os.environ['PIPE_STDOUT_FILE'], 'r') as f: + stdout_msg = f.read() else: stdout_msg = "act.py: stdout: executed act.py %s\\n" % ' '.join(sys.argv[1:]) sys.stdout.write( stdout_msg ) if 'PIPE_STDERR_FILE' in os.environ: - stderr_msg = open(os.environ['PIPE_STDERR_FILE'], 'r').read() + with open(os.environ['PIPE_STDERR_FILE'], 'r') as f: + stderr_msg = f.read() else: stderr_msg = "act.py: stderr: executed act.py %s\\n" % ' '.join(sys.argv[1:]) - sys.stderr.write( stderr_msg ) + sys.stderr.write(stderr_msg) sys.exit(0) """) diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py index 1e544a1..c9068ed 100644 --- a/src/engine/SCons/BuilderTests.py +++ b/src/engine/SCons/BuilderTests.py @@ -680,7 +680,7 @@ class BuilderTestCase(unittest.TestCase): def test_single_source(self): """Test Builder with single_source flag set""" def func(target, source, env): - open(str(target[0]), "w") + open(str(target[0]), "w") # TODO: this just a throwaway? if (len(source) == 1 and len(target) == 1): env['CNT'][0] = env['CNT'][0] + 1 @@ -736,10 +736,12 @@ class BuilderTestCase(unittest.TestCase): """Testing handling lists of targets and source""" def function2(target, source, env, tlist = [outfile, outfile2], **kw): for t in target: - open(str(t), 'w').write("function2\n") + with open(str(t), 'w') as f: + f.write("function2\n") for t in tlist: if not t in list(map(str, target)): - open(t, 'w').write("function2\n") + with open(t, 'w') as f: + f.write("function2\n") return 1 env = Environment() @@ -765,10 +767,12 @@ class BuilderTestCase(unittest.TestCase): def function3(target, source, env, tlist = [sub1_out, sub2_out]): for t in target: - open(str(t), 'w').write("function3\n") + with open(str(t), 'w') as f: + f.write("function3\n") for t in tlist: if not t in list(map(str, target)): - open(t, 'w').write("function3\n") + with open(t, 'w') as f: + f.write("function3\n") return 1 builder = SCons.Builder.Builder(action = function3) diff --git a/src/engine/SCons/SConfTests.py b/src/engine/SCons/SConfTests.py index cf8a7fb..73fcf8d 100644 --- a/src/engine/SCons/SConfTests.py +++ b/src/engine/SCons/SConfTests.py @@ -298,12 +298,14 @@ int main(void) { """Test SConf.TryAction """ def actionOK(target, source, env): - open(str(target[0]), "w").write("RUN OK\n") + with open(str(target[0]), "w") as f: + f.write("RUN OK\n") return None def actionFAIL(target, source, env): return 1 def actionUnicode(target, source, env): - open(str(target[0]), "wb").write('2\302\242\n') + with open(str(target[0]), "wb") as f: + f.write('2\302\242\n') return None diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py index 42ed00e..c0c77b0 100644 --- a/src/engine/SCons/TaskmasterTests.py +++ b/src/engine/SCons/TaskmasterTests.py @@ -1085,6 +1085,7 @@ class TaskmasterTestCase(unittest.TestCase): exception_values = [ "integer division or modulo", "integer division or modulo by zero", + "integer division by zero", # PyPy2 ] assert str(exc_value) in exception_values, exc_value diff --git a/src/engine/SCons/cppTests.py b/src/engine/SCons/cppTests.py index ebf7fde..0b346e8 100644 --- a/src/engine/SCons/cppTests.py +++ b/src/engine/SCons/cppTests.py @@ -817,7 +817,8 @@ class fileTestCase(unittest.TestCase): return '\n'.join(map(strip_spaces, lines)) def write(self, file, contents): - open(file, 'w').write(self.strip_initial_spaces(contents)) + with open(file, 'w') as f: + f.write(self.strip_initial_spaces(contents)) def test_basic(self): """Test basic file inclusion""" diff --git a/test/Batch/generated.py b/test/Batch/generated.py index 21b8250..65ce8a8 100644 --- a/test/Batch/generated.py +++ b/test/Batch/generated.py @@ -36,10 +36,12 @@ test = TestSCons.TestSCons() test.write('SConstruct', """ def batch_build(target, source, env): for t, s in zip(target, source): - fp = open(str(t), 'wb') - if str(t) == 'f3.out': - fp.write(open('f3.include', 'rb').read()) - fp.write(open(str(s), 'rb').read()) + with open(str(t), 'wb') as fp: + if str(t) == 'f3.out': + with open('f3.include', 'rb') as f: + fp.write(f.read()) + with open(str(s), 'rb') as f: + fp.write(f.read()) env = Environment() bb = Action(batch_build, batch_key=True) env['BUILDERS']['Batch'] = Builder(action=bb) diff --git a/test/CacheDir/NoCache.py b/test/CacheDir/NoCache.py index f0929aa..8ecfeb3 100644 --- a/test/CacheDir/NoCache.py +++ b/test/CacheDir/NoCache.py @@ -46,7 +46,8 @@ CacheDir(r'%s') g = '%s' def ActionWithUndeclaredInputs(target,source,env): - open(target[0].get_abspath(),'w').write(g) + with open(target[0].get_abspath(),'w') as f: + f.write(g) Command('foo_cached', [], ActionWithUndeclaredInputs) NoCache(Command('foo_notcached', [], ActionWithUndeclaredInputs)) diff --git a/test/Deprecated/TargetSignatures/build-content.py b/test/Deprecated/TargetSignatures/build-content.py index 6a5a8c4..efdaaee 100644 --- a/test/Deprecated/TargetSignatures/build-content.py +++ b/test/Deprecated/TargetSignatures/build-content.py @@ -47,7 +47,8 @@ SetOption('warn', 'deprecated-target-signatures') env = Environment() def copy1(env, source, target): - open(str(target[0]), 'wb').write(open(str(source[0]), 'rb').read()) + with open(str(target[0]), 'wb') as fo, open(str(source[0]), 'rb') as fi: + fo.write(fi.read()) def copy2(env, source, target): %s diff --git a/test/Fortran/link-with-cxx.py b/test/Fortran/link-with-cxx.py index a29558e..bf10fc8 100644 --- a/test/Fortran/link-with-cxx.py +++ b/test/Fortran/link-with-cxx.py @@ -48,7 +48,8 @@ elif sys.argv[1][:5] == '/OUT:': outfile = open(sys.argv[1][5:], 'wb') infiles = sys.argv[2:] for infile in infiles: - outfile.write(open(infile, 'rb').read()) + with open(infile, 'rb') as f: + outfile.write(f.read()) outfile.close() sys.exit(0) """) @@ -69,7 +70,8 @@ import SCons.Tool.link def copier(target, source, env): s = str(source[0]) t = str(target[0]) - open(t, 'wb').write(open(s, 'rb').read()) + with open(t, 'wb') as fo, open(s, 'rb') as fi: + fo.write(fi.read()) env = Environment(CXX = r'%(_python_)s test_linker.py', CXXCOM = Action(copier), SMARTLINK = SCons.Tool.link.smart_link, diff --git a/test/MSVC/generate-rc.py b/test/MSVC/generate-rc.py index 00e9090..3dd4331 100644 --- a/test/MSVC/generate-rc.py +++ b/test/MSVC/generate-rc.py @@ -39,16 +39,16 @@ fake_rc = test.workpath('fake_rc.py') test.write(fake_rc, """\ import sys -contents = open(sys.argv[2], 'r').read() -open(sys.argv[1], 'w').write("fake_rc.py\\n" + contents) +with open(sys.argv[1], 'w') as fo, open(sys.argv[2], 'r') as fi: + fo.write("fake_rc.py\\n" + fi.read()) """) test.write('SConstruct', """ def generate_rc(target, source, env): t = str(target[0]) s = str(source[0]) - tfp = open(t, 'w') - tfp.write('generate_rc\\n' + open(s, 'r').read()) + with open(t, 'w') as fo, open(s, 'r') as fi: + fo.write('generate_rc\\n' + fi.read()) env = Environment(tools=['msvc'], RCCOM=r'%(_python_)s %(fake_rc)s $TARGET $SOURCE') diff --git a/test/NodeOps.py b/test/NodeOps.py index 1f856c3..99a3f6a 100644 --- a/test/NodeOps.py +++ b/test/NodeOps.py @@ -122,7 +122,8 @@ import os Import('*') def mycopy(env, source, target): - open(str(target[0]),'w').write(open(str(source[0]),'r').read()) + with open(str(target[0]), 'wt') as fo, open(str(source[0]), 'rt') as fi: + fo.write(fi.read()) def exists_test(node): before = os.path.exists(str(node)) # doesn't exist yet in VariantDir diff --git a/test/Repository/Local.py b/test/Repository/Local.py index 95fd898..7062075 100644 --- a/test/Repository/Local.py +++ b/test/Repository/Local.py @@ -49,7 +49,8 @@ def copy(env, source, target): source = str(source[0]) target = str(target[0]) print('copy() < %s > %s' % (source, target)) - open(target, "w").write(open(source, "r").read()) + with open(target, 'w') as fo, open(source, 'r') as fi: + fo.write(fi.read()) Build = Builder(action=copy) env = Environment(BUILDERS={'Build':Build}, BBB='bbb') @@ -66,7 +67,8 @@ test.write(['repository', 'src', 'SConscript'], r""" def bbb_copy(env, source, target): target = str(target[0]) print('bbb_copy()') - open(target, "w").write(open('build/bbb.1', "r").read()) + with open(target, 'w') as fo, open('build/bbb.1', 'r') as fi: + fo.write(fi.read()) Import("env") env.Build('bbb.1', 'bbb.0') diff --git a/test/Repository/option-c.py b/test/Repository/option-c.py index b0d8533..58c4876 100644 --- a/test/Repository/option-c.py +++ b/test/Repository/option-c.py @@ -66,7 +66,8 @@ def copy(env, source, target): source = str(source[0]) target = str(target[0]) print('copy() < %s > %s' % (source, target)) - open(target, "w").write(open(source, "r").read()) + with open(target, 'w') as fo, open(source, 'r') as fi: + fo.write(fi.read()) Build = Builder(action=copy) env = Environment(BUILDERS={'Build':Build}) diff --git a/test/Requires/eval-order.py b/test/Requires/eval-order.py index 696b5e9..77fbc98 100644 --- a/test/Requires/eval-order.py +++ b/test/Requires/eval-order.py @@ -34,11 +34,13 @@ test = TestSCons.TestSCons() test.write('SConstruct', """ def copy_and_create_func(target, source, env): - fp = open(str(target[0]), 'w') - for s in source: - fp.write(open(str(s), 'r').read()) - fp.close() + with open(str(target[0]), 'w') as fp: + for s in source: + with open(str(s), 'r') as f: + fp.write(f.read()) open('file.in', 'w').write("file.in 1\\n") + with open('file.in', 'w') as f: + f.write("file.in 1\\n") return None copy_and_create = Action(copy_and_create_func) env = Environment() diff --git a/test/Value.py b/test/Value.py index 34b036b..5a6a48e 100644 --- a/test/Value.py +++ b/test/Value.py @@ -48,7 +48,8 @@ L = len(P) C = Custom(P) def create(target, source, env): - open(str(target[0]), 'wb').write(source[0].get_contents()) + with open(str(target[0]), 'wb') as f: + f.write(source[0].get_contents()) env = Environment() env['BUILDERS']['B'] = Builder(action = create) @@ -62,7 +63,9 @@ def create_value (target, source, env): target[0].write(source[0].get_contents()) def create_value_file (target, source, env): - open(str(target[0]), 'wb').write(source[0].read()) + #open(str(target[0]), 'wb').write(source[0].read()) + with open(str(target[0]), 'wb') as f: + f.write(source[0].read()) env['BUILDERS']['B2'] = Builder(action = create_value) env['BUILDERS']['B3'] = Builder(action = create_value_file) @@ -75,7 +78,8 @@ env.B3('f5.out', V) test.write('put.py', """\ import os import sys -open(sys.argv[-1],'w').write(" ".join(sys.argv[1:-2])) +with open(sys.argv[-1],'w') as f: + f.write(" ".join(sys.argv[1:-2])) """) # Run all of the tests with both types of source signature diff --git a/test/VariantDir/File-create.py b/test/VariantDir/File-create.py index 25e6c09..ad9234b 100644 --- a/test/VariantDir/File-create.py +++ b/test/VariantDir/File-create.py @@ -49,12 +49,14 @@ SConscript('src/SConscript', variant_dir='build1', chdir=1, duplicate=1) test.write(['src', 'SConscript'], """\ #f1_in = File('f1.in') #Command('f1.out', f1_in, Copy('$TARGET', '$SOURCE')) -#open('f1.in', 'w').write("f1.in\\n") +#with open('f1.in', 'w') as f: +# f.write("f1.in\\n") f2_in = File('f2.in') str(f2_in) Command('f2.out', f2_in, Copy('$TARGET', '$SOURCE')) -open('f2.in', 'w').write("f2.in\\n") +with open('f2.in', 'w') as f: + f.write("f2.in\\n") """) test.run(arguments = '--tree=all .') diff --git a/test/chained-build.py b/test/chained-build.py index c3351e1..871a593 100644 --- a/test/chained-build.py +++ b/test/chained-build.py @@ -38,7 +38,8 @@ test.subdir('w1') SConstruct1_contents = """\ def build(env, target, source): - open(str(target[0]), 'wt').write(open(str(source[0]), 'rt').read()) + with open(str(target[0]), 'wt') as fo, open(str(source[0]), 'rt') as fi: + fo.write(fi.read()) env=Environment(BUILDERS={'B' : Builder(action=build)}) env.B('foo.mid', 'foo.in') @@ -46,7 +47,8 @@ env.B('foo.mid', 'foo.in') SConstruct2_contents = """\ def build(env, target, source): - open(str(target[0]), 'wt').write(open(str(source[0]), 'rt').read()) + with open(str(target[0]), 'wt') as fo, open(str(source[0]), 'rt') as fi: + fo.write(fi.read()) env=Environment(BUILDERS={'B' : Builder(action=build)}) env.B('foo.out', 'foo.mid') diff --git a/test/implicit-cache/basic.py b/test/implicit-cache/basic.py index b7cd984..c03a320 100644 --- a/test/implicit-cache/basic.py +++ b/test/implicit-cache/basic.py @@ -65,7 +65,8 @@ env = Environment(CPPPATH=['inc2', include]) SConscript('variant/SConscript', "env") def copy(target, source, env): - open(str(target[0]), 'wt').write(open(str(source[0]), 'rt').read()) + with open(str(target[0]), 'wt') as fo, open(str(source[0]), 'rt') as fi: + fo.write(fi.read()) nodep = env.Command('nodeps.c', 'nodeps.in', action=copy) env.Program('nodeps', 'nodeps.c') diff --git a/test/sconsign/nonwritable.py b/test/sconsign/nonwritable.py index 6f12f18..812a476 100644 --- a/test/sconsign/nonwritable.py +++ b/test/sconsign/nonwritable.py @@ -51,13 +51,15 @@ work2_sub3__sconsign = test.workpath('work2', 'sub3', '.sconsign') SConstruct_contents = """\ def build1(target, source, env): - open(str(target[0]), 'wb').write(open(str(source[0]), 'rb').read()) + with open(str(target[0]), 'wb') as fo, open(str(source[0]), 'rb') as fi: + fo.write(fi.read()) return None def build2(target, source, env): import os import os.path - open(str(target[0]), 'wb').write(open(str(source[0]), 'rb').read()) + with open(str(target[0]), 'wb') as fo, open(str(source[0]), 'rb') as fi: + fo.write(fi.read()) dir, file = os.path.split(str(target[0])) os.chmod(dir, 0o555) return None @@ -92,8 +94,10 @@ test.write(['work2', 'SConstruct'], SConstruct_contents) test.write(['work2', 'foo.in'], "work2/foo.in\n") -pickle.dump({}, open(work2_sub1__sconsign, 'wb'), 1) -pickle.dump({}, open(work2_sub2__sconsign, 'wb'), 1) +with open(work2_sub1__sconsign, 'wb') as p: + pickle.dump({}, p, 1) +with open(work2_sub2__sconsign, 'wb') as p: + pickle.dump({}, p, 1) os.chmod(work2_sub1__sconsign, 0o444) diff --git a/testing/framework/TestSCons.py b/testing/framework/TestSCons.py index 38ffd08..7fe48e8 100644 --- a/testing/framework/TestSCons.py +++ b/testing/framework/TestSCons.py @@ -920,7 +920,8 @@ for opt, arg in cmd_opts: else: opt_string = opt_string + ' ' + opt output.write("/* mymoc.py%s */\\n" % opt_string) for a in args: - contents = open(a, 'r').read() + with open(a, 'r') as f: + contents = f.read() a = a.replace('\\\\', '\\\\\\\\') subst = r'{ my_qt_symbol( "' + a + '\\\\n" ); }' if impl: diff --git a/testing/framework/TestSCons_time.py b/testing/framework/TestSCons_time.py index bc116ec..6299f51 100644 --- a/testing/framework/TestSCons_time.py +++ b/testing/framework/TestSCons_time.py @@ -24,8 +24,7 @@ from TestCommon import __all__ # some of the scons_time tests may need regex-based matching: from TestSCons import search_re, search_re_in_list -__all__.extend([ 'TestSCons_time', - ]) +__all__.extend(['TestSCons_time',]) SConstruct = """\ from __future__ import print_function @@ -37,25 +36,30 @@ scons_py = """\ #!/usr/bin/env python import os import sys + def write_args(fp, args): fp.write(args[0] + '\\n') for arg in args[1:]: fp.write(' ' + arg + '\\n') + write_args(sys.stdout, sys.argv) for arg in sys.argv[1:]: if arg[:10] == '--profile=': - profile = open(arg[10:], 'w') - profile.write('--profile\\n') - write_args(profile, sys.argv) + with open(arg[10:], 'w') as profile: + profile.write('--profile\\n') + write_args(profile, sys.argv) break sys.stdout.write('SCONS_LIB_DIR = ' + os.environ['SCONS_LIB_DIR'] + '\\n') -exec(open('SConstruct', 'r').read()) +with open('SConstruct', 'r') as f: + script = f.read() +exec(script) """ aegis_py = """\ #!/usr/bin/env python import os import sys + script_dir = 'src/script' if not os.path.exists(script_dir): os.makedirs(script_dir) @@ -68,6 +72,20 @@ svn_py = """\ #!/usr/bin/env python import os import sys + +dir = sys.argv[-1] +script_dir = dir + '/src/script' +os.makedirs(script_dir) +open(script_dir + '/scons.py', 'w').write( +r'''%s''') +""" % scons_py + + +git_py = """\ +#!/usr/bin/env python +import os +import sys + dir = sys.argv[-1] script_dir = dir + '/src/script' os.makedirs(script_dir) @@ -239,6 +257,12 @@ class TestSCons_time(TestCommon): os.chmod(name, 0o755) return name + def write_fake_git_py(self, name): + name = self.workpath(name) + self.write(name, git_py) + os.chmod(name, 0o755) + return name + def write_sample_directory(self, archive, dir, files): dir = self.workpath(dir) for name, content in files: -- cgit v0.12