From 6e958b8415f850b23fb5404b54ec1af59e6a56bd Mon Sep 17 00:00:00 2001 From: valeros Date: Tue, 22 Dec 2020 00:21:32 +0200 Subject: [PATCH] Handle possible issues when check tool cannot be executed // Resolve #3753 Now, each tool individually decides under what conditions the check is considered failed. --- platformio/commands/check/tools/base.py | 29 ++++++++++++++++---- platformio/commands/check/tools/clangtidy.py | 6 ++++ platformio/commands/check/tools/cppcheck.py | 13 +++++---- platformio/commands/check/tools/pvsstudio.py | 18 ++++++++---- tests/commands/test_check.py | 16 +++++++++++ 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/platformio/commands/check/tools/base.py b/platformio/commands/check/tools/base.py index dc9f476f..7eda6936 100644 --- a/platformio/commands/check/tools/base.py +++ b/platformio/commands/check/tools/base.py @@ -167,6 +167,29 @@ class CheckToolBase(object): # pylint: disable=too-many-instance-attributes if os.path.isfile(f): os.remove(f) + @staticmethod + def is_check_successful(cmd_result): + return cmd_result["returncode"] == 0 + + def execute_check_cmd(self, cmd): + result = proc.exec_command( + cmd, + stdout=proc.LineBufferedAsyncPipe(self.on_tool_output), + stderr=proc.LineBufferedAsyncPipe(self.on_tool_output), + ) + + if not self.is_check_successful(result): + click.echo( + "\nError: Failed to execute check command! Exited with code %d." + % result["returncode"] + ) + if self.options.get("verbose"): + click.echo(result["out"]) + click.echo(result["err"]) + self._bad_input = True + + return result + @staticmethod def get_project_target_files(patterns): c_extension = (".c",) @@ -200,11 +223,7 @@ class CheckToolBase(object): # pylint: disable=too-many-instance-attributes if self.options.get("verbose"): click.echo(" ".join(cmd)) - proc.exec_command( - cmd, - stdout=proc.LineBufferedAsyncPipe(self.on_tool_output), - stderr=proc.LineBufferedAsyncPipe(self.on_tool_output), - ) + self.execute_check_cmd(cmd) else: if self.options.get("verbose"): diff --git a/platformio/commands/check/tools/clangtidy.py b/platformio/commands/check/tools/clangtidy.py index 06f3ff76..3206cdba 100644 --- a/platformio/commands/check/tools/clangtidy.py +++ b/platformio/commands/check/tools/clangtidy.py @@ -49,6 +49,12 @@ class ClangtidyCheckTool(CheckToolBase): return DefectItem(severity, category, message, file_, line, column, defect_id) + @staticmethod + def is_check_successful(cmd_result): + # Note: Clang-Tidy returns 1 for not critical compilation errors, + # so 0 and 1 are only acceptable values + return cmd_result["returncode"] < 2 + def configure_command(self): tool_path = join(get_core_package_dir("tool-clangtidy"), "clang-tidy") diff --git a/platformio/commands/check/tools/cppcheck.py b/platformio/commands/check/tools/cppcheck.py index b38bb8d6..46d15d07 100644 --- a/platformio/commands/check/tools/cppcheck.py +++ b/platformio/commands/check/tools/cppcheck.py @@ -109,7 +109,7 @@ class CppcheckCheckTool(CheckToolBase): cmd = [ tool_path, "--addon-python=%s" % proc.get_pythonexe_path(), - "--error-exitcode=1", + "--error-exitcode=3", "--verbose" if self.options.get("verbose") else "--quiet", ] @@ -220,6 +220,11 @@ class CppcheckCheckTool(CheckToolBase): if os.path.isfile(dump_file): os.remove(dump_file) + @staticmethod + def is_check_successful(cmd_result): + # Cppcheck is configured to return '3' if a defect is found + return cmd_result["returncode"] in (0, 3) + def check(self, on_defect_callback=None): self._on_defect_callback = on_defect_callback project_files = self.get_project_target_files(self.options["patterns"]) @@ -238,11 +243,7 @@ class CppcheckCheckTool(CheckToolBase): if self.options.get("verbose"): click.echo(" ".join(cmd)) - proc.exec_command( - cmd, - stdout=proc.LineBufferedAsyncPipe(self.on_tool_output), - stderr=proc.LineBufferedAsyncPipe(self.on_tool_output), - ) + self.execute_check_cmd(cmd) self.clean_up() diff --git a/platformio/commands/check/tools/pvsstudio.py b/platformio/commands/check/tools/pvsstudio.py index ce5d93ec..88b1a513 100644 --- a/platformio/commands/check/tools/pvsstudio.py +++ b/platformio/commands/check/tools/pvsstudio.py @@ -52,6 +52,11 @@ class PvsStudioCheckTool(CheckToolBase): # pylint: disable=too-many-instance-at ) ) + def tool_output_filter(self, line): + if "license was not entered" in line.lower(): + self._bad_input = True + return line + def _process_defects(self, defects): for defect in defects: if not isinstance(defect, DefectItem): @@ -203,6 +208,12 @@ class PvsStudioCheckTool(CheckToolBase): # pylint: disable=too-many-instance-at if os.path.isdir(self._tmp_dir): shutil.rmtree(self._tmp_dir) + @staticmethod + def is_check_successful(cmd_result): + return ( + "license" not in cmd_result["err"].lower() and cmd_result["returncode"] == 0 + ) + def check(self, on_defect_callback=None): self._on_defect_callback = on_defect_callback for scope, files in self.get_project_target_files( @@ -219,11 +230,8 @@ class PvsStudioCheckTool(CheckToolBase): # pylint: disable=too-many-instance-at self._bad_input = True continue - result = proc.exec_command(cmd) - # pylint: disable=unsupported-membership-test - if result["returncode"] != 0 or "license" in result["err"].lower(): - self._bad_input = True - click.echo(result["err"]) + result = self.execute_check_cmd(cmd) + if result["returncode"] != 0: continue self._process_defects(self.parse_defects(self._tmp_output_file)) diff --git a/tests/commands/test_check.py b/tests/commands/test_check.py index 06c133fc..b8c8e65a 100644 --- a/tests/commands/test_check.py +++ b/tests/commands/test_check.py @@ -410,6 +410,22 @@ check_tool = pvs-studio assert style == 0 +def test_check_pvs_studio_fails_without_license(clirunner, tmpdir): + config = DEFAULT_CONFIG + "\ncheck_tool = pvs-studio" + + tmpdir.join("platformio.ini").write(config) + tmpdir.mkdir("src").join("main.c").write(TEST_CODE) + + default_result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir)]) + verbose_result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir), "-v"]) + + assert default_result.exit_code != 0 + assert "failed to perform check" in default_result.output.lower() + + assert verbose_result.exit_code != 0 + assert "license was not entered" in verbose_result.output.lower() + + def test_check_embedded_platform_all_tools(clirunner, validate_cliresult, tmpdir): config = """ [env:test]