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

@@ -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(
+            self.execute_check_cmd(cmd)
-                cmd,
-                stdout=proc.LineBufferedAsyncPipe(self.on_tool_output),
-                stderr=proc.LineBufferedAsyncPipe(self.on_tool_output),
-            )
 
         else:
             if self.options.get("verbose"):

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")
 

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(
+            self.execute_check_cmd(cmd)
-                cmd,
-                stdout=proc.LineBufferedAsyncPipe(self.on_tool_output),
-                stderr=proc.LineBufferedAsyncPipe(self.on_tool_output),
-            )
 
         self.clean_up()
 

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)
+                result = self.execute_check_cmd(cmd)
-                # pylint: disable=unsupported-membership-test
+                if result["returncode"] != 0:
-                if result["returncode"] != 0 or "license" in result["err"].lower():
-                    self._bad_input = True
-                    click.echo(result["err"])
                     continue
 
                 self._process_defects(self.parse_defects(self._tmp_output_file))

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]