From 4693200968b98c42d331dc4748d32afc846fed77 Mon Sep 17 00:00:00 2001 From: "Somsikov, Andrey" Date: Tue, 23 Jun 2020 12:24:08 +0300 Subject: [PATCH 1/4] Limit log file name length to OS allowed limit --- gtest_parallel.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/gtest_parallel.py b/gtest_parallel.py index b0032ba..8f17fec 100755 --- a/gtest_parallel.py +++ b/gtest_parallel.py @@ -44,6 +44,11 @@ import fcntl +# The log file names is truncated to 256 to overcome OS limitations +MKSTEMP_RANDOM_LEN = 12 # length of mkstemp random string being added +NAMEMAX_DEFAULT = 255 # Limit for log file name length + + # An object that catches SIGINT sent to the Python process and notices # if processes passed to wait() die by SIGINT (we need to look for # both of those cases, because pressing Ctrl+C can result in either @@ -204,14 +209,18 @@ def _logname(output_dir, test_binary, test_name, execution_number): # Store logs to temporary files if there is no output_dir. if output_dir is None: (log_handle, log_name) = tempfile.mkstemp(prefix='gtest_parallel_', - suffix=".log") - os.close(log_handle) - return log_name - - log_name = '%s-%s-%d.log' % (Task._normalize(os.path.basename(test_binary)), - Task._normalize(test_name), execution_number) + suffix='.log') + else: + suffix = '-%d.log' % execution_number + prefix = '%s-%s' % (Task._normalize(os.path.basename(test_binary)), + Task._normalize(test_name)) + namemax = os.statvfs('.').f_namemax if hasattr(os, 'statvfs') else NAMEMAX_DEFAULT + (log_handle, log_name) = tempfile.mkstemp(prefix=prefix[:namemax - MKSTEMP_RANDOM_LEN - len(suffix)], + dir=output_dir, + suffix=suffix) + os.close(log_handle) + return log_name - return os.path.join(output_dir, log_name) def run(self): begin = time.time() From ce97ee70960aa8364d922720d11f228f7c4bffc0 Mon Sep 17 00:00:00 2001 From: "Somsikov, Andrey" Date: Fri, 17 Jul 2020 02:35:17 +0300 Subject: [PATCH 2/4] Attempt creating log file before trimming --- gtest_parallel.py | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/gtest_parallel.py b/gtest_parallel.py index 8f17fec..afd871d 100755 --- a/gtest_parallel.py +++ b/gtest_parallel.py @@ -44,11 +44,6 @@ import fcntl -# The log file names is truncated to 256 to overcome OS limitations -MKSTEMP_RANDOM_LEN = 12 # length of mkstemp random string being added -NAMEMAX_DEFAULT = 255 # Limit for log file name length - - # An object that catches SIGINT sent to the Python process and notices # if processes passed to wait() die by SIGINT (we need to look for # both of those cases, because pressing Ctrl+C can result in either @@ -206,19 +201,38 @@ def _normalize(string): @staticmethod def _logname(output_dir, test_binary, test_name, execution_number): + # The log file names is truncated to 256 to overcome OS limitations + MKSTEMP_RANDOM_LEN = 12 # length of mkstemp random string being added + MAX_PREFIX_LENGTH = 240 # Limit for file name mkstemp prefix + # Store logs to temporary files if there is no output_dir. if output_dir is None: (log_handle, log_name) = tempfile.mkstemp(prefix='gtest_parallel_', suffix='.log') + os.close(log_handle) else: - suffix = '-%d.log' % execution_number prefix = '%s-%s' % (Task._normalize(os.path.basename(test_binary)), Task._normalize(test_name)) - namemax = os.statvfs('.').f_namemax if hasattr(os, 'statvfs') else NAMEMAX_DEFAULT - (log_handle, log_name) = tempfile.mkstemp(prefix=prefix[:namemax - MKSTEMP_RANDOM_LEN - len(suffix)], - dir=output_dir, - suffix=suffix) - os.close(log_handle) + suffix = '-%d.log' % execution_number + log_name = os.path.join(output_dir, prefix + suffix) + try: + # check if log can be created or exists already + if not os.path.exists(log_name): + os.makedirs(output_dir, exist_ok=True) + with open(log_name, 'w+'): + pass + os.remove(log_name) + except OSError as os_error: + # truncate file name if error is Errno 36: File name too long + if os_error.errno == 36: + prefix_length = ((os.statvfs('.').f_namemax - + MKSTEMP_RANDOM_LEN) if hasattr(os, 'statvfs') else MAX_PREFIX_LENGTH) - len(suffix) + (log_handle, log_name) = tempfile.mkstemp(prefix=prefix[:prefix_length], + dir=output_dir, + suffix=suffix) + os.close(log_handle) + else: + raise return log_name From 8ed9e962c4e03c1db37cb95262808fbc8639a68f Mon Sep 17 00:00:00 2001 From: "Somsikov, Andrey" Date: Fri, 17 Jul 2020 02:39:46 +0300 Subject: [PATCH 3/4] Add test for log file trimming Also change root for test_log_file_names tests to be a temporary directory to allow runnig tests without sudo. The old root `/` or `C:` is not usually writable for regular users. --- gtest_parallel_tests.py | 68 +++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/gtest_parallel_tests.py b/gtest_parallel_tests.py index a8ff350..8cb0d30 100755 --- a/gtest_parallel_tests.py +++ b/gtest_parallel_tests.py @@ -311,36 +311,44 @@ def test_times_worker(): class TestTask(unittest.TestCase): def test_log_file_names(self): - def root(): - return 'C:\\' if sys.platform == 'win32' else '/' - - self.assertEqual( - os.path.join('.', 'bin-Test_case-100.log'), - gtest_parallel.Task._logname('.', 'bin', 'Test.case', 100)) - - self.assertEqual( - os.path.join('..', 'a', 'b', 'bin-Test_case_2-1.log'), - gtest_parallel.Task._logname(os.path.join('..', 'a', 'b'), - os.path.join('..', 'bin'), - 'Test.case/2', 1)) - - self.assertEqual( - os.path.join('..', 'a', 'b', 'bin-Test_case_2-5.log'), - gtest_parallel.Task._logname(os.path.join('..', 'a', 'b'), - os.path.join(root(), 'c', 'd', 'bin'), - 'Test.case/2', 5)) - - self.assertEqual( - os.path.join(root(), 'a', 'b', 'bin-Instantiation_Test_case_2-3.log'), - gtest_parallel.Task._logname(os.path.join(root(), 'a', 'b'), - os.path.join('..', 'c', 'bin'), - 'Instantiation/Test.case/2', 3)) - - self.assertEqual( - os.path.join(root(), 'a', 'b', 'bin-Test_case-1.log'), - gtest_parallel.Task._logname(os.path.join(root(), 'a', 'b'), - os.path.join(root(), 'c', 'd', 'bin'), - 'Test.case', 1)) + with tempfile.TemporaryDirectory() as root: + self.assertEqual( + os.path.join('.', 'bin-Test_case-100.log'), + gtest_parallel.Task._logname('.', 'bin', 'Test.case', 100)) + + self.assertEqual( + os.path.join('..', 'a', 'b', 'bin-Test_case_2-1.log'), + gtest_parallel.Task._logname(os.path.join('..', 'a', 'b'), + os.path.join('..', 'bin'), + 'Test.case/2', 1)) + + self.assertEqual( + os.path.join('..', 'a', 'b', 'bin-Test_case_2-5.log'), + gtest_parallel.Task._logname(os.path.join('..', 'a', 'b'), + os.path.join(root, 'c', 'd', 'bin'), + 'Test.case/2', 5)) + + self.assertEqual( + os.path.join(root, 'a', 'b', 'bin-Instantiation_Test_case_2-3.log'), + gtest_parallel.Task._logname(os.path.join(root, 'a', 'b'), + os.path.join('..', 'c', 'bin'), + 'Instantiation/Test.case/2', 3)) + + self.assertEqual( + os.path.join(root, 'a', 'b', 'bin-Test_case-1.log'), + gtest_parallel.Task._logname(os.path.join(root, 'a', 'b'), + os.path.join(root, 'c', 'd', 'bin'), + 'Test.case', 1)) + MAX_PREFIX_LENGTH = 240 + long_test_name = 'a' * (os.statvfs('.').f_namemax if hasattr(os, + 'statvfs') else MAX_PREFIX_LENGTH + 1) + truncated_log = gtest_parallel.Task._logname(os.path.join(root, 'out'), + os.path.join(root, 'bin'), + long_test_name, 1) + self.assertNotEqual('bin-' + long_test_name + '-1.log', os.path.basename(truncated_log)) + self.assertTrue(truncated_log.endswith('-1.log')) + self.assertTrue(truncated_log.startswith(os.path.join( + root, 'out', 'bin-' + long_test_name[:MAX_PREFIX_LENGTH-len('bin--1.log')]))) def test_logs_to_temporary_files_without_output_dir(self): log_file = gtest_parallel.Task._logname(None, None, None, None) From 3728b54769bcddbd22b18f398e4b9c6b0870e83d Mon Sep 17 00:00:00 2001 From: "Somsikov, Andrey" Date: Fri, 17 Jul 2020 11:17:54 +0300 Subject: [PATCH 4/4] Cleanup logname stripping comments --- gtest_parallel.py | 56 ++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/gtest_parallel.py b/gtest_parallel.py index afd871d..96ca851 100755 --- a/gtest_parallel.py +++ b/gtest_parallel.py @@ -201,38 +201,40 @@ def _normalize(string): @staticmethod def _logname(output_dir, test_binary, test_name, execution_number): - # The log file names is truncated to 256 to overcome OS limitations - MKSTEMP_RANDOM_LEN = 12 # length of mkstemp random string being added - MAX_PREFIX_LENGTH = 240 # Limit for file name mkstemp prefix - # Store logs to temporary files if there is no output_dir. if output_dir is None: (log_handle, log_name) = tempfile.mkstemp(prefix='gtest_parallel_', suffix='.log') os.close(log_handle) - else: - prefix = '%s-%s' % (Task._normalize(os.path.basename(test_binary)), - Task._normalize(test_name)) - suffix = '-%d.log' % execution_number - log_name = os.path.join(output_dir, prefix + suffix) - try: - # check if log can be created or exists already - if not os.path.exists(log_name): - os.makedirs(output_dir, exist_ok=True) - with open(log_name, 'w+'): - pass - os.remove(log_name) - except OSError as os_error: - # truncate file name if error is Errno 36: File name too long - if os_error.errno == 36: - prefix_length = ((os.statvfs('.').f_namemax - - MKSTEMP_RANDOM_LEN) if hasattr(os, 'statvfs') else MAX_PREFIX_LENGTH) - len(suffix) - (log_handle, log_name) = tempfile.mkstemp(prefix=prefix[:prefix_length], - dir=output_dir, - suffix=suffix) - os.close(log_handle) - else: - raise + return log_name + + # The log file name is a combination of a test binary name and a test name. + # If the log file name exceeds OS limitations it will be truncated. + MKSTEMP_RANDOM_LEN = 12 # length of mkstemp random string being added + MAX_PREFIX_LENGTH = 240 # Limit for file name mkstemp prefix + + prefix = '%s-%s' % (Task._normalize(os.path.basename(test_binary)), + Task._normalize(test_name)) + suffix = '-%d.log' % execution_number + log_name = os.path.join(output_dir, prefix + suffix) + try: + # check if log can be created or exists already + if not os.path.exists(log_name): + os.makedirs(output_dir, exist_ok=True) + with open(log_name, 'w+'): + pass + os.remove(log_name) + except OSError as os_error: + # truncate file name if error is Errno 36: File name too long + if os_error.errno == 36: + prefix_length = ((os.statvfs('.').f_namemax - + MKSTEMP_RANDOM_LEN) if hasattr(os, 'statvfs') else MAX_PREFIX_LENGTH) - len(suffix) + (log_handle, log_name) = tempfile.mkstemp(prefix=prefix[:prefix_length], + dir=output_dir, + suffix=suffix) + os.close(log_handle) + else: + raise return log_name