From 0a2f0d61e47cf28e84791059b92c804d6e485594 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 8 Jan 2020 19:00:51 -0800 Subject: [PATCH 1/4] Cleanup file_packager python code Also clean up the test code (test change should be NFC). --- tests/test_other.py | 14 ++++++++------ tools/file_packager.py | 43 +++++++++++++++++++++--------------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/tests/test_other.py b/tests/test_other.py index 5392b4e8a64f5..ac67ee7823d25 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -8,7 +8,6 @@ from __future__ import print_function from functools import wraps -import filecmp import glob import itertools import json @@ -2543,21 +2542,24 @@ def test_file_packager(self): self.assertNotContained('below the current directory', proc2.stderr) def clean(txt): - return [line for line in txt.split('\n') if 'PACKAGE_UUID' not in line and 'loadPackage({' not in line] + lines = txt.splitlines() + lines = [l for l in lines if 'PACKAGE_UUID' not in l and 'loadPackage({' not in l] + return ''.join(lines) - assert clean(proc.stdout) == clean(proc2.stdout) + self.assertTextDataIdentical(clean(proc.stdout), clean(proc2.stdout)) # verify '--separate-metadata' option produces separate metadata file os.chdir('..') run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata']) - assert os.path.isfile('immutable.js.metadata') + self.assertExists('immutable.js.metadata') # verify js output file is immutable when metadata is separated shutil.copy2('immutable.js', 'immutable.js.copy') # copy with timestamp preserved + time.sleep(3.0) run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata']) - assert filecmp.cmp('immutable.js.copy', 'immutable.js') # assert both file content and timestamp are the same as reference copy - self.assertEqual(str(os.path.getmtime('immutable.js.copy')), str(os.path.getmtime('immutable.js'))) + self.assertTextDataIdentical(open('immutable.js.copy').read(), open('immutable.js').read()) + self.assertEqual(os.path.getmtime('immutable.js.copy'), os.path.getmtime('immutable.js')) # verify the content of metadata file is correct with open('immutable.js.metadata') as f: metadata = json.load(f) diff --git a/tools/file_packager.py b/tools/file_packager.py index bc434b764a059..5ffe32e028cb1 100755 --- a/tools/file_packager.py +++ b/tools/file_packager.py @@ -232,7 +232,8 @@ def main(): from_emcc = True leading = '' elif arg.startswith('--plugin'): - plugin = open(arg.split('=', 1)[1], 'r').read() + with open(arg.split('=', 1)[1]) as f: + plugin = f.read() eval(plugin) # should append itself to plugins leading = '' elif leading == 'preload' or leading == 'embed': @@ -391,17 +392,18 @@ def was_seen(name): if has_preloaded: # Bundle all datafiles into one archive. Avoids doing lots of simultaneous # XHRs which has overhead. - data = open(data_target, 'wb') start = 0 - for file_ in data_files: - file_['data_start'] = start - curr = open(file_['srcpath'], 'rb').read() - file_['data_end'] = start + len(curr) - if AV_WORKAROUND: - curr += '\x00' - start += len(curr) - data.write(curr) - data.close() + with open(data_target, 'wb') as data: + for file_ in data_files: + file_['data_start'] = start + with open(file_['srcpath'], 'rb') as f: + curr = f.read() + file_['data_end'] = start + len(curr) + if AV_WORKAROUND: + curr += '\x00' + start += len(curr) + data.write(curr) + # TODO: sha256sum on data_target if start > 256 * 1024 * 1024: print('warning: file packager is creating an asset bundle of %d MB. ' @@ -913,20 +915,17 @@ def was_seen(name): # differs from the current generated one, otherwise leave the file # untouched preserving its old timestamp if os.path.isfile(jsoutput): - f = open(jsoutput, 'r+') - old = f.read() + with open(jsoutput) as f: + old = f.read() if old != ret: - f.seek(0) - f.write(ret) - f.truncate() + with open(jsoutput, 'w') as f: + f.write(ret) else: - f = open(jsoutput, 'w') - f.write(ret) - f.close() + with open(jsoutput, 'w') as f: + f.write(ret) if separate_metadata: - f = open(jsoutput + '.metadata', 'w') - json.dump(metadata, f, separators=(',', ':')) - f.close() + with open(jsoutput + '.metadata', 'w') as f: + json.dump(metadata, f, separators=(',', ':')) return 0 From 70e5effa89d6a6acb5c5c1048526825baf9943f8 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 9 Jan 2020 10:50:30 -0800 Subject: [PATCH 2/4] revert --- tests/test_other.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_other.py b/tests/test_other.py index ac67ee7823d25..2e09b07f36f7e 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -2555,7 +2555,6 @@ def clean(txt): self.assertExists('immutable.js.metadata') # verify js output file is immutable when metadata is separated shutil.copy2('immutable.js', 'immutable.js.copy') # copy with timestamp preserved - time.sleep(3.0) run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata']) # assert both file content and timestamp are the same as reference copy self.assertTextDataIdentical(open('immutable.js.copy').read(), open('immutable.js').read()) From a12774b68d8928b34bde72deed8ca6fe961d6525 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 9 Jan 2020 11:03:51 -0800 Subject: [PATCH 3/4] sleep --- tests/test_other.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_other.py b/tests/test_other.py index 2e09b07f36f7e..4c04a0b8bafa0 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -2553,8 +2553,11 @@ def clean(txt): run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata']) self.assertExists('immutable.js.metadata') - # verify js output file is immutable when metadata is separated + # verify js output JS file is not touched when the metadata is separated shutil.copy2('immutable.js', 'immutable.js.copy') # copy with timestamp preserved + # ensure some time passes before running the packager again so that if it does touch the + # js file it will end up with the different timestamp. + time.sleep(1.0) run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata']) # assert both file content and timestamp are the same as reference copy self.assertTextDataIdentical(open('immutable.js.copy').read(), open('immutable.js').read()) From 712ae829dea22a9fda286af4c28ee1e566c76dfe Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 9 Jan 2020 11:37:46 -0800 Subject: [PATCH 4/4] flake8 --- tests/test_other.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_other.py b/tests/test_other.py index 4c04a0b8bafa0..f46980c18e4b9 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -2555,7 +2555,7 @@ def clean(txt): self.assertExists('immutable.js.metadata') # verify js output JS file is not touched when the metadata is separated shutil.copy2('immutable.js', 'immutable.js.copy') # copy with timestamp preserved - # ensure some time passes before running the packager again so that if it does touch the + # ensure some time passes before running the packager again so that if it does touch the # js file it will end up with the different timestamp. time.sleep(1.0) run_process([PYTHON, FILE_PACKAGER, 'test.data', '--preload', 'data1.txt', '--preload', 'subdir/data2.txt', '--js-output=immutable.js', '--separate-metadata'])