Repository: MarketSquare/robotframework-robocop (python)

Issue:
robocop format --check does not seem to return correct exit code
Used robocop version 6.0.2

0 exit code is returned even files would be reformatted:

$ robocop format --check --line-length 140
Would reformat robot/resources/SEC/crl.resource
Would reformat robot/resources/SEC/gnmi.resource

2 files would be reformatted, 75 files would be left unchanged.
$ echo $?
0

Using robotidy this returns exit code 1 and according to documentation I
understand that "robocop format" should work similarly.


Pull request description:
Fix robocop format --check always returning 0
Fixes #1371

Patch:
diff --git a/docs/releasenotes/6.0.3.rst b/docs/releasenotes/6.0.3.rst
index bbcf160..e14ff86 100644
--- a/docs/releasenotes/6.0.3.rst
+++ b/docs/releasenotes/6.0.3.rst
@@ -9,6 +9,15 @@ Enabling reports in configuration does not work (#1369)
 
 Options common for whole run (such as reports) should be now loaded correctly from the configuration file.
 
+robocop format --check always return 0 (#1371)
+----------------------------------------------
+
+``--check`` should now return correct exit code:
+
+- 1 if ``--check`` is used and there are modified files
+- 0 otherwise
+- 2 on invalid configuration, robocop exceptions
+
 Acknowledgements
 ================
 
diff --git a/src/robocop/formatter/runner.py b/src/robocop/formatter/runner.py
index 9089d10..06f4036 100644
--- a/src/robocop/formatter/runner.py
+++ b/src/robocop/formatter/runner.py
@@ -3,8 +3,9 @@ from __future__ import annotations
 import os
 import sys
 from difflib import unified_diff
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, NoReturn
 
+import typer
 from rich import console
 from robot.api import get_model
 from robot.errors import DataError
@@ -69,7 +70,7 @@ class RobocopFormatter:
                 skipped_files += 1
         return self.formatting_result(all_files, changed_files, skipped_files, stdin)
 
-    def formatting_result(self, all_files: int, changed_files: int, skipped_files: int, stdin: bool) -> int:
+    def formatting_result(self, all_files: int, changed_files: int, skipped_files: int, stdin: bool) -> NoReturn:
         """Print formatting summary and return status code."""
         if not stdin:
             all_files = all_files - changed_files - skipped_files
@@ -83,9 +84,11 @@ class RobocopFormatter:
                 f"{all_files} file{all_files_plurar}{future_tense} left unchanged."
                 + (f" {skipped_files} file{skipped_files_plurar}{future_tense} skipped." if skipped_files else "")
             )
-        if not self.config_manager.default_config.formatter.check or not changed_files:
-            return 0
-        return 1  # FIXME: ensure proper exit status is returned
+        if changed_files and self.config_manager.default_config.formatter.check:
+            exit_code = 1
+        else:
+            exit_code = 0
+        raise typer.Exit(code=exit_code)
 
     def format_until_stable(self, model: File):
         disabler_finder = disablers.RegisterDisablers(self.config.formatter.start_line, self.config.formatter.end_line)


Test patch:
diff --git a/tests/formatter/__init__.py b/tests/formatter/__init__.py
index a416920..aa511ea 100644
--- a/tests/formatter/__init__.py
+++ b/tests/formatter/__init__.py
@@ -4,6 +4,7 @@ import filecmp
 from difflib import unified_diff
 from pathlib import Path
 
+import click
 import pytest
 from packaging import version
 from packaging.specifiers import SpecifierSet
@@ -75,7 +76,7 @@ class FormatterAcceptanceTest:
         select: list[str] | None = None,
         configure: list[str] | None = None,
         source: str | None = None,
-        # exit_code: int = 0, TODO
+        exit_code: int = 0,
         not_modified: bool = False,
         test_on_version: str | None = None,
         **kwargs,
@@ -87,22 +88,20 @@ class FormatterAcceptanceTest:
             source_path = self.FORMATTERS_DIR / self.FORMATTER_NAME / "source"
         else:
             source_path = self.FORMATTERS_DIR / self.FORMATTER_NAME / "source" / source
-        format_files(
-            sources=[source_path],
-            select=select,
-            configure=configure,
-            overwrite=True,
-            check=not_modified,
-            output=output_path,
-            line_ending="unix",
-            **kwargs,
-        )
-        # if result.exit_code != exit_code:
-        #     raise AssertionError(
-        #         f"robotidy exit code: {result.exit_code} does not match expected: {exit_code}. "
-        #         f"Exception description: {result.exception}"
-        #     )
-        # return result
+        with pytest.raises(click.exceptions.Exit) as exc_info:
+            format_files(
+                sources=[source_path],
+                select=select,
+                configure=configure,
+                overwrite=True,
+                check=not_modified,
+                output=output_path,
+                line_ending="unix",
+                **kwargs,
+            )
+        if exit_code is not None:
+            assert exc_info.value.exit_code == exit_code
+        return exc_info
 
     def compare_file(self, actual_name: str, expected_name: str | None = None):
         if expected_name is None:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index df5a3cd..33a9fa5 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1,5 +1,8 @@
 """Test CLI commands / options common for linter and formatter."""
 
+from pathlib import Path
+
+import pytest
 from typer.testing import CliRunner
 
 from robocop import __version__
@@ -91,3 +94,25 @@ class TestListFormatters:
         assert "Generated SARIF report at .sarif.json" in result.stdout
         assert "Generated JSON report at robocop.json" in result.stdout
         assert "Generated SonarQube report at robocop_sonar_qube.json" in result.stdout
+
+    @pytest.mark.parametrize(
+        ("check", "will_format", "expected_exit_code"),
+        [
+            (False, True, 0),
+            (False, False, 0),
+            (True, True, 1),
+            (True, False, 0),
+        ],
+    )
+    def test_exit_code(self, check, will_format, expected_exit_code):
+        test_data = Path(__file__).parent / "formatter" / "formatters" / "NormalizeSeparators"
+        if will_format:
+            test_data = test_data / "source"
+        else:
+            test_data = test_data / "expected"
+        command = ["format", "--select", "NormalizeSeparators", "--no-overwrite"]
+        if check:
+            command += ["--check"]
+        with working_directory(test_data):
+            result = CliRunner().invoke(app, [*command, "test.robot"])
+        assert result.exit_code == expected_exit_code