Repository: MarketSquare/robotframework-robocop
Pull request #1259 (2025-02-27 09:04:18)

Add tests with extended view for ARG02
I have extended our test framework to support adding tests for extended format & added tests for ARG02 already.


Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 300a8a1..604616b 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,5 +1,76 @@
 ---- Document under rework -----
 
+Testing
+=======
+
+We are using pytest for our test framework. While it would be possible to use Robot Framework, it's easier for us
+to use different test framework as we can easily mock in unit tests and setup different robot framework versions
+for acceptance testing.
+
+Our tests structure around main features in Robocop:
+
+tests/
+    formatter/ - formatting tests
+    linter/ - linting tests
+    config/ - config related tests (note that some of the config related tests may be still inside linter/formatter)
+
+Typically, users are contributing in following areas:
+
+- adding / updating existing rules
+- adding / updating reports
+- adding / updating formatters
+- core functions changes
+
+We will cover how to approach testing for each category.
+
+Adding / updating existing rules
+--------------------------------
+
+After you update or add new rule, or even better before you do it, you need to create tests for it under
+``tests/linter/rules/<rule_category>/<rule_name>``. Rule name has replaced ``-` characters with ``_`` so it's
+recognized by default by pytest.
+
+For example rule ``invalid_argument`` has tests placed under ``tests/linter/rules/arguments/invalid_argument``
+directory.
+
+Each such directory has at least following files: 
+
+- empty ``__init__.py`` file (for tests discovery)
+- ``test_rule.py`` file that describes tests
+- ``test.robot`` file (name can be different) with Robot Framework code to be linted
+- ``expected_output.txt`` with content of output which Robocop is expected to print
+
+Depending on tested features you may have multiple robot files with one expected file, or several expected files
+(for example one for every robot version, if rule behaviour change over versions).
+
+Example Python file:
+
+```Python
+from tests.linter.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(
+            src_files=["test.robot"], expected_file="expected_output.txt", test_on_version=">=4"
+        )
+
+```
+
+It will select and run only one rule (name will be taken from the directory name) on ``test.robot`` file, compare
+output with ``expected_output.txt``. Tests will only run on versions ``>=4``.
+
+Default output format in Robocop is ``extended`` view. Most tests use ``simple`` view by default (for simplicity). 
+It's highly recommended to check how rule behaves with both views. You can configure to run on different view
+using ``configure``:
+
+```Python
+def test_rule(self):
+    self.check_rule(
+        src_files=["test.robot"], expected_file="expected_output.txt", output_format="extended"
+    )
+```
+
 Documentation
 =============
 
diff --git a/src/robocop/linter/reports/print_issues.py b/src/robocop/linter/reports/print_issues.py
index b56c0d5..6f56b67 100644
--- a/src/robocop/linter/reports/print_issues.py
+++ b/src/robocop/linter/reports/print_issues.py
@@ -115,6 +115,9 @@ class PrintIssuesReport(robocop.linter.reports.Report):
     def _gutter(line_no: int | str, gutter_width: int, indent: str):
         return f"[cyan]{line_no:>{gutter_width}} |[/cyan]{indent}"
 
+    def _print_line(self, line:str) -> None:
+        self.console.print(line.rstrip())
+
     def _print_issue_with_lines(self, lines: list[str], source_rel_path: Path, diagnostic: Diagnostic) -> None:
         """
         Print diagnostic information for a specific range of lines in a source file.
@@ -151,26 +154,26 @@ class PrintIssuesReport(robocop.linter.reports.Report):
             for line_no in range(start_line - 2, start_line):
                 if line_no < 1:
                     continue
-                self.console.print(f"{self._gutter(line_no, gutter_width, indent)} {escape(lines[line_no-1])}")
+                self._print_line(f"{self._gutter(line_no, gutter_width, indent)} {escape(lines[line_no-1])}")
         # issue
         if start_line == end_line:  # error in one line (most cases)
-            self.console.print(f"{self._gutter(start_line, gutter_width, indent)} {escape(lines[start_line-1])}")
-            self.console.print(
+            self._print_line(f"{self._gutter(start_line, gutter_width, indent)} {escape(lines[start_line-1])}")
+            self._print_line(
                 f"{self._gutter(' ', gutter_width, indent)} "
                 f"[red]{' '* (start_col - 1)}{'^'* max(end_col - start_col, 1)} {diagnostic.rule.rule_id}[/red]"
             )
         else:  # multi line errors, such as SPC05
             for line in range(start_line, end_line + 1):
                 sep = "/" if line == start_line else "|"
-                self.console.print(
+                self._print_line(
                     f"{self._gutter(line, gutter_width, indent='')} [red]{sep}[/red] {escape(lines[line - 1])}"
                 )
-            self.console.print(f"{self._gutter(' ', gutter_width, indent='')} [red]|_^ {diagnostic.rule.rule_id}[/red]")
+            self._print_line(f"{self._gutter(' ', gutter_width, indent='')} [red]|_^ {diagnostic.rule.rule_id}[/red]")
         # code after issue
         for line_no in range(end_line + 1, end_line + 3):
             if line_no > len(lines) or not lines[line_no - 1].strip():
                 break
-            self.console.print(f"{self._gutter(line_no, gutter_width, indent)} {escape(lines[line_no-1])}")
+            self._print_line(f"{self._gutter(line_no, gutter_width, indent)} {escape(lines[line_no-1])}")
         self.console.print(self._gutter(" ", gutter_width, indent))
         print()
 


Test patch:
diff --git a/tests/linter/rules/arguments/argument_overwritten_before_usage/expected_extended.txt b/tests/linter/rules/arguments/argument_overwritten_before_usage/expected_extended.txt
new file mode 100644
index 0000000..46ebc6b
--- /dev/null
+++ b/tests/linter/rules/arguments/argument_overwritten_before_usage/expected_extended.txt
@@ -0,0 +1,60 @@
+test.robot:22:31 ARG02 Keyword argument '${overwritten1}' is overwritten before usage
+    |
+ 20 |
+ 21 | Overwritten Argument
+ 22 |     [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten3}    ${overwritten_but_used}
+    |                               ^^^^^^^^^^^^^^^ ARG02
+ 23 |     ${overwritten1}    Set Variable    ${arg1}
+ 24 |     ${overwritten2}    Set Variable    ${arg1}
+    |
+
+test.robot:22:50 ARG02 Keyword argument '${overwritten2}' is overwritten before usage
+    |
+ 20 |
+ 21 | Overwritten Argument
+ 22 |     [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten3}    ${overwritten_but_used}
+    |                                                  ^^^^^^^^^^^^^^^ ARG02
+ 23 |     ${overwritten1}    Set Variable    ${arg1}
+ 24 |     ${overwritten2}    Set Variable    ${arg1}
+    |
+
+test.robot:22:69 ARG02 Keyword argument '${overwritten3}' is overwritten before usage
+    |
+ 20 |
+ 21 | Overwritten Argument
+ 22 |     [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten3}    ${overwritten_but_used}
+    |                                                                     ^^^^^^^^^^^^^^^ ARG02
+ 23 |     ${overwritten1}    Set Variable    ${arg1}
+ 24 |     ${overwritten2}    Set Variable    ${arg1}
+    |
+
+test.robot:39:20 ARG02 Keyword argument '${error}' is overwritten before usage
+    |
+ 37 |
+ 38 | Overwritten In Try As
+ 39 |     [Arguments]    ${error}    ${used}
+    |                    ^^^^^^^^ ARG02
+ 40 |     TRY
+ 41 |         Do Stuff
+    |
+
+test.robot:70:31 ARG02 Keyword argument '${overwritten1}' is overwritten before usage
+    |
+ 68 |
+ 69 | Overwritten in VAR
+ 70 |     [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten_but_used}
+    |                               ^^^^^^^^^^^^^^^ ARG02
+ 71 |     VAR    ${overwritten1}    ${arg1}
+ 72 |     VAR    ${overwritten2}    ${arg1}
+    |
+
+test.robot:70:50 ARG02 Keyword argument '${overwritten2}' is overwritten before usage
+    |
+ 68 |
+ 69 | Overwritten in VAR
+ 70 |     [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten_but_used}
+    |                                                  ^^^^^^^^^^^^^^^ ARG02
+ 71 |     VAR    ${overwritten1}    ${arg1}
+ 72 |     VAR    ${overwritten2}    ${arg1}
+    |
+
diff --git a/tests/linter/rules/arguments/argument_overwritten_before_usage/test_rule.py b/tests/linter/rules/arguments/argument_overwritten_before_usage/test_rule.py
index 9e33407..bef9d1f 100644
--- a/tests/linter/rules/arguments/argument_overwritten_before_usage/test_rule.py
+++ b/tests/linter/rules/arguments/argument_overwritten_before_usage/test_rule.py
@@ -28,3 +28,11 @@ class TestRuleAcceptance(RuleAcceptance):
 
     def test_rule_arguments_should_clear_after_keyword(self):
         self.check_rule(src_files=["arguments_cleared_after_keyword.robot"], expected_file=None)
+
+    def test_extended_view(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_extended.txt",
+            test_on_version=">=7",
+            output_format="extended"
+        )
diff --git a/tests/linter/utils/__init__.py b/tests/linter/utils/__init__.py
index 4ed8cc0..b69b65c 100644
--- a/tests/linter/utils/__init__.py
+++ b/tests/linter/utils/__init__.py
@@ -6,10 +6,13 @@ import os
 import re
 import sys
 from pathlib import Path
+from unittest import mock
 from typing import TYPE_CHECKING
 
 import click.exceptions
+from difflib import unified_diff
 import pytest
+from rich.console import Console
 
 from robocop.cli import check_files
 from robocop.linter.utils.misc import ROBOT_VERSION
@@ -26,7 +29,9 @@ def isolated_output():
     bytes_output = io.BytesIO()
     sys.stdout = io.TextIOWrapper(bytes_output, encoding="utf-8")
     sys.stderr = sys.stdout
-    yield bytes_output
+    # By default, pytest uses 80-chars long terminal which depends on COLUMNS env variable
+    with mock.patch.dict(os.environ, {"COLUMNS": "200"}, clear=True):
+        yield bytes_output
     sys.stdout = old_stdout
     sys.stderr = old_stderr
 
@@ -50,25 +55,38 @@ def get_result(encoded_output):
     return convert_to_output(encoded_output.getvalue())
 
 
-def normalize_result(result, test_data):
+def normalize_result(result, test_data, sort_lines: bool):
     """Remove test data directory path from paths and sort the lines."""
     test_data_str = f"{test_data}{os.path.sep}"
-    return sorted([line.replace(test_data_str, "") for line in result])
+    lines = [line.replace(test_data_str, "") for line in result]
+    if sort_lines:
+        return sorted(lines)
+    return lines
 
-
-def load_expected_file(test_data, expected_file):
+def load_expected_file(test_data, expected_file, sort_lines: bool):
     if expected_file is None:
         return []
     expected = test_data / expected_file
     with open(expected, encoding="utf-8") as f:
-        return sorted(
-            [
+        lines =             [
                 re.sub(r"(?<!\\)(?:\\\\)*(\${/})", os.path.sep.replace("\\", "\\\\"), line.rstrip("\n")).replace(
                     "\\${/}", "${/}"
                 )
                 for line in f
             ]
-        )
+        if sort_lines:
+            return sorted(lines)
+        return lines
+
+
+def display_lines_diff(expected: list[str], actual: list[str]):
+    lines = list(
+        unified_diff(expected, actual, fromfile=f"expected:\t", tofile=f"actual:\t")
+    )
+    # colorized_output = decorate_diff_with_color(lines)
+    console = Console(color_system="auto", width=400)
+    for line in lines:
+        console.print(line, highlight=False)
 
 
 class RuleAcceptance:
@@ -87,13 +105,15 @@ class RuleAcceptance:
         test_on_version: str | list[str] | None = None,
         issue_format: str = "default",
         language: list[str] | None = None,
+        output_format: str = "simple",
         # deprecated: bool = False, TODO
         **kwargs,
     ):
         if not self.enabled_in_version(test_on_version):
             pytest.skip(f"Test enabled only for RF {test_on_version}")
         test_data = self.test_class_dir
-        expected = load_expected_file(test_data, expected_file)
+        sort_lines = output_format=="simple"
+        expected = load_expected_file(test_data, expected_file, sort_lines=sort_lines)
         issue_format = self.get_issue_format(issue_format)
         if select is None:
             select = [self.rule_name]
@@ -101,6 +121,9 @@ class RuleAcceptance:
             paths = [test_data]
         else:
             paths = [test_data / src_file for src_file in src_files]
+        if configure is None:
+            configure = []
+        configure.append(f"print_issues.output_format={output_format}")
         with isolated_output() as output, working_directory(test_data):
             try:
                 with pytest.raises(click.exceptions.Exit):
@@ -117,20 +140,24 @@ class RuleAcceptance:
                 sys.stdout.flush()
                 result = get_result(output)
                 parsed_results = result.splitlines()
-        actual = normalize_result(parsed_results, test_data)
+        actual = normalize_result(parsed_results, test_data, sort_lines=sort_lines)
         # if deprecated:  # TODO:
         #     assert runner.rules[self.rule_name].deprecation_warning in actual
         # elif
         if actual != expected:
-            missing_expected = sorted(set(actual) - set(expected))
-            missing_actual = sorted(set(expected) - set(actual))
             error = "Actual issues are different than expected.\n"
-            if missing_expected:
-                present_in_actual = "\n    ".join(missing_expected)
-                error += f"Actual issues not found in expected:\n    {present_in_actual}\n\n"
-            if missing_actual:
-                present_in_expected = "\n    ".join(missing_actual)
-                error += f"Expected issues not found in actual:\n    {present_in_expected}"
+            if sort_lines:
+                missing_expected = sorted(set(actual) - set(expected))
+                missing_actual = sorted(set(expected) - set(actual))
+                error = "Actual issues are different than expected.\n"
+                if missing_expected:
+                    present_in_actual = "\n    ".join(missing_expected)
+                    error += f"Actual issues not found in expected:\n    {present_in_actual}\n\n"
+                if missing_actual:
+                    present_in_expected = "\n    ".join(missing_actual)
+                    error += f"Expected issues not found in actual:\n    {present_in_expected}"
+            else:
+                display_lines_diff(expected, actual)
             pytest.fail(error, pytrace=False)
 
     def get_issue_format(self, issue_format):