From a3bff213cc5acae8f5238f767234900dd0425182 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Jul 2023 14:10:02 +0200 Subject: [PATCH 1/6] Fix use of the is operator on a string It worked as intended because the string is a literal and CPython (which our CI uses) merges string literals at compile time. But it was fragile with respect to code changes or using another Python implementation. This fixes the diagnostic emitted by python when running the script: ``` windows_testing.py:185: SyntaxWarning: "is not" with a literal. Did you mean "!="? if name is not "Results": ``` Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index 7f6f4e8ae..4916bdcae 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -182,7 +182,7 @@ def setup_logger(self, name, log_file, level=logging.INFO): ) console = logging.StreamHandler() file_handler = logging.FileHandler(log_file) - if name is not "Results": + if name != "Results": console.setFormatter(log_formatter) file_handler.setFormatter(log_formatter) logger = logging.getLogger(name) From 3c5ed809a4d10d3479dc366fc6fc592d0a134cbe Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Jul 2023 14:16:03 +0200 Subject: [PATCH 2/6] Fix anomalous backslash in strings Fix regex strings missing an r prefix, and a docstring containing what was intended to be a literal backslash. Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index 4916bdcae..ffceb5604 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -149,12 +149,12 @@ def __init__(self, self.mingw_result = None self.solution_file_pattern = r"(?i)mbed *TLS\.sln\Z" self.visual_studio_build_success_patterns = [ - "Build succeeded.", "\d+ Warning\(s\)", "0 Error\(s\)" + r"Build succeeded.", r"\d+ Warning\(s\)", r"0 Error\(s\)" ] self.visual_studio_build_zero_warnings_string = "0 Warning(s)" - self.selftest_success_pattern = "\[ All tests (PASS|passed) \]" - self.test_suites_success_pattern = "100% tests passed, 0 tests failed" - self.mingw_success_pattern = "PASSED \(\d+ suites, \d+ tests run\)" + self.selftest_success_pattern = r"\[ All tests (PASS|passed) \]" + self.test_suites_success_pattern = r"100% tests passed, 0 tests failed" + self.mingw_success_pattern = r"PASSED \(\d+ suites, \d+ tests run\)" self.config_pl_location = os.path.join("scripts", "config.pl") self.selftest_exe = "selftest.exe" self.mingw_command = "mingw32-make" @@ -426,7 +426,7 @@ def run_test_suites_on_built_code(self, solution_dir, test_run, logger): def get_environment_containing_VSCMD_START_DIR(self, solution_dir): """This is done to bypass a 'feature' added in Visual Studio 2017. - If the %USERPROFILE%\Source directory exists, then running + If the %USERPROFILE%\\Source directory exists, then running vcvarsall.bat will silently change the directory to that directory. Setting the VSCMD_START_DIR environment variable causes it to change to that directory instead""" From 7153f9540df73dc5a2e850a9aa01f36645f10da4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Jul 2023 14:20:20 +0200 Subject: [PATCH 3/6] Improve error backtraces When re-raising an exception, show the original backtrace, independently of any logging. Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index ffceb5604..d1cc36ce1 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -218,7 +218,7 @@ def get_clean_worktree_for_git_reference(self, logger): except subprocess.CalledProcessError as error: self.set_return_code(2) logger.error(error.output) - raise Exception("Checking out worktree failed, aborting") + raise Exception("Checking out worktree failed, aborting") from error def cleanup_git_worktree(self, git_worktree_path, logger): shutil.rmtree(git_worktree_path) @@ -235,7 +235,7 @@ def cleanup_git_worktree(self, git_worktree_path, logger): except subprocess.CalledProcessError as error: self.set_return_code(2) logger.error(error.output) - raise Exception("Worktree cleanup failed, aborting") + raise Exception("Worktree cleanup failed, aborting") from error def set_config_on_code(self, git_worktree_path, logger): """Enables all config specified in config.pl, then disables config @@ -267,7 +267,7 @@ def set_config_on_code(self, git_worktree_path, logger): except subprocess.CalledProcessError as error: self.set_return_code(2) logger.error(error.output) - raise Exception("Setting config failed, aborting") + raise Exception("Setting config failed, aborting") from error def generate_seedfile(self, filename): """This tests if a file exists, and if not, creates it with 64 bytes @@ -524,7 +524,7 @@ def build_visual_studio_solution_using_cmake(self, except subprocess.CalledProcessError as error: self.set_return_code(2) logger.error(error.output) - raise Exception("Building solution using Cmake failed, aborting") + raise Exception("Building solution using Cmake failed, aborting") from error def generate_source_files(self, git_worktree_path, logger): """Generate configuration-independent source files if required.""" @@ -551,7 +551,7 @@ def generate_source_files(self, git_worktree_path, logger): except subprocess.CalledProcessError as error: self.set_return_code(2) logger.error(error.output) - raise Exception("{} failed, aborting".format(batch_script)) + raise Exception("{} failed, aborting".format(batch_script)) from error def test_visual_studio_built_code(self, test_run, solution_type): log_name = "VS{} {} {}{} {}".format( From a5b3d8501cc3a5cdf02ccd51b17b6eab41a0df66 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Jul 2023 14:24:45 +0200 Subject: [PATCH 4/6] Tell pylint we mean it when continuing after an exception In all of these cases, one test in a series has failed, so we make a note of the failure and continue. Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index d1cc36ce1..7779ae377 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -308,7 +308,7 @@ def test_mingw_built_code(self): ) if not self.mingw_result: self.set_return_code(1) - except Exception as error: + except Exception as error: #pylint: disable=broad-except self.set_return_code(2) mingw_logger.error(error) traceback.print_exc() @@ -617,7 +617,7 @@ def test_visual_studio_built_code(self, test_run, solution_type): selftest_code_path, vs_logger ) test_run.results[solution_type + " selftest"] = selftest_result - except Exception as error: + except Exception as error: #pylint: disable=broad-except vs_logger.error(error) traceback.print_exc() self.set_return_code(2) @@ -701,7 +701,7 @@ def run_all_tests(self): self.test_visual_studio_built_code( vs_test_run, solution_type ) - except Exception: + except Exception: #pylint: disable=broad-except traceback.print_exc() self.set_return_code(2) finally: From 50035b10eaa09dec5781ded0a9e770d82202803e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Jul 2023 14:35:02 +0200 Subject: [PATCH 5/6] Pacify a Pylint warning Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index 7779ae377..b716685ba 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -480,7 +480,11 @@ def build_code_using_visual_studio(self, "msbuild /nodeReuse:false /t:Rebuild /p:Configuration={},Platform={}," "PlatformToolset={} /m \"{}\"\n".format( test_run.configuration, test_run.architecture, - retarget, solution_file + retarget, + # The `for solution_file ...` loop has an else clause with + # an unconditional return, so solution_file is always defined + # by this point. Pylint doesn't realize that. + solution_file #pylint: disable=undefined-loop-variable ) ) msbuild_process.stdin.close() From da94cd91268b1b4c28690c84b0713d22b8c2fabb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 10 Aug 2023 16:25:39 +0200 Subject: [PATCH 6/6] Fix . in regex Signed-off-by: Gilles Peskine --- resources/windows/windows_testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/windows/windows_testing.py b/resources/windows/windows_testing.py index b716685ba..4ff7a43bb 100644 --- a/resources/windows/windows_testing.py +++ b/resources/windows/windows_testing.py @@ -149,7 +149,7 @@ def __init__(self, self.mingw_result = None self.solution_file_pattern = r"(?i)mbed *TLS\.sln\Z" self.visual_studio_build_success_patterns = [ - r"Build succeeded.", r"\d+ Warning\(s\)", r"0 Error\(s\)" + r"Build succeeded\.", r"\d+ Warning\(s\)", r"0 Error\(s\)" ] self.visual_studio_build_zero_warnings_string = "0 Warning(s)" self.selftest_success_pattern = r"\[ All tests (PASS|passed) \]"