Restructure aspect integration tests

.bazelrc

@@ -1,10 +1,18 @@
 # Always tell why tests fail
 test --test_output=errors
 
-# We use Bazel's modern dependency management system. This works for us only with Bazel >= 6.2.0
-# This is the deprecated version. "--enable_bzlmod" is the forward path. However, as Bazel < 6.0.0 does not support the
-# new flag. We keep using the deprecated one until we no longer support Bazel 5.
-build --experimental_enable_bzlmod=true
+# We use Bazel's modern dependency management system for development.
+build --enable_bzlmod=true
+
+# We are not yet sure if we really want to lock the bzlmod resolution down given we test with various Bazel versions
+# and configurations. It seems the main benefits of the lock file are not having to reanalyze the central registry
+# when working without a cached workspace and being safeguarded against changed or yanked modules in the central
+# registry. Both don't matter much to us right now.
+build --lockfile_mode=off
+
+# When working with hermetic Python toolchains, supporting the legacy runfiles layout is needlessly wasting resources.
+# See https://github.com/bazelbuild/rules_python/issues/1653
+build --nolegacy_external_runfiles
 
 # Mypy integration
 build:mypy --aspects=@mypy_integration//:mypy.bzl%mypy_aspect

.github/workflows/ci.yaml

@@ -28,7 +28,7 @@ jobs:
       - name: Unit tests
         run: bazel test -- //src/... //test/aspect:all //third_party/...
       - name: Execute Mypy
-        run: bazel build --config=mypy -- //src/... //test/aspect:all
+        run: bazel build --config=mypy -- //src/... //test/aspect:all //test/apply_fixes:all
 
   integration-tests-aspect:
     runs-on: ubuntu-22.04

README.md

@@ -103,7 +103,7 @@ You can invoke the aspect from within a rule.
 This can be useful to make the execution part of a bazel build without having to manually execute the longish aspect build command.
 
 The Bazel documentation for invoking an aspect from within a rule can be found [here](https://bazel.build/rules/aspects#invoking_the_aspect_from_a_rule).
-A concrete example for doing so for the DWYU aspect can be found in [a rule in the recursion test cases](test/aspect/recursion/rule.bzl).
+A concrete example for doing so for the DWYU aspect can be found in [aspect integration tests](test/aspect/rule_using_aspect).
 
 # Configuring DWYU
 

ruff.toml

@@ -40,8 +40,8 @@ select = [
 ignore = [
     # It is fine tha some line are longer than the length limit
     "E501",
+    # Number of function arguments is too opinionated
+    "PLR0913",
     # High false positive rate. Often a plain number is the most expressive, e.g. when checling lengths.
     "PLR2004",
-    # Using Optinal and '= None' at once is overly verbose and hurts readability
-    "RUF013",
 ]

test/apply_fixes/BUILD

@@ -0,0 +1,7 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+# Helper target to enable mypy
+py_library(
+    name = "python_files",
+    srcs = glob(["**/*.py"]),
+)

test/apply_fixes/execution_logic.py

@@ -6,6 +6,7 @@
 from tempfile import TemporaryDirectory
 from typing import List, Optional
 
+from result import Error
 from test_case import TestCaseBase
 
 TEST_CASES_DIR = Path("test/apply_fixes")
@@ -58,29 +59,32 @@ def cleanup(test_workspace: Path) -> None:
 
 
 def execute_test(test: TestCaseBase, origin_workspace: Path) -> bool:
-    logging.info(f">>> Executing '{test.name}'")
+    logging.info(f">>> Test '{test.name}'")
     succeeded = False
     result = None
 
     with TemporaryDirectory() as temporary_workspace:
+        workspace_path = Path(temporary_workspace)
         try:
             setup_test_workspace(
                 origin_workspace=origin_workspace,
                 test_sources=test.test_sources,
                 extra_workspace_file_content=test.extra_workspace_file_content,
-                temporary_workspace=Path(temporary_workspace),
+                temporary_workspace=workspace_path,
             )
-            result = test.execute_test(Path(temporary_workspace))
+            result = test.execute_test(workspace_path)
         except Exception:
             logging.exception("Test failed due to exception:")
 
-        cleanup(temporary_workspace)
+        cleanup(workspace_path)
 
-    if result is not None:
-        if not result.is_success():
-            logging.info(result.error)
-        else:
-            succeeded = True
+    if result is None:
+        result = Error("No result")
+
+    if result.is_success():
+        succeeded = True
+    else:
+        logging.info(result.error)
 
     logging.info(f'<<< {"OK" if succeeded else "FAILURE"}\n')
 

test/apply_fixes/test_case.py

@@ -11,7 +11,7 @@ class TestCaseBase(ABC):
     def __init__(self, name: str, test_sources: Path) -> None:
         self._name = name
         self._test_sources = test_sources
-        self._workspace = None
+        self._workspace = Path("")
 
     #
     # Interface
@@ -64,25 +64,44 @@ def _create_reports(
         """
         Create report files as input for the applying fixes script
         """
-        cmd = ["bazel"]
-        if startup_args:
-            cmd.extend(startup_args)
-        cmd.extend(["build", f"--aspects=//:aspect.bzl%{aspect}", "--output_groups=dwyu"])
-        if extra_args:
-            cmd.extend(extra_args)
-        cmd.append(self.test_target)
-        self._run_cmd(cmd=cmd, check=False)
-
-    def _run_automatic_fix(self, extra_args: List[str] = None) -> None:
+        cmd_startup_args = startup_args if startup_args else []
+        cmd_extra_args = extra_args if extra_args else []
+
+        self._run_cmd(
+            cmd=[
+                "bazel",
+                *cmd_startup_args,
+                "build",
+                "--nolegacy_external_runfiles",
+                f"--aspects=//:aspect.bzl%{aspect}",
+                "--output_groups=dwyu",
+                *cmd_extra_args,
+                "--",
+                self.test_target,
+            ],
+            check=False,
+        )
+
+    def _run_automatic_fix(self, extra_args: Optional[List[str]] = None) -> None:
         """
         Execute the applying fixes script for the Bazel target associated with the test case
         """
-        cmd = ["bazel", "run", "@depend_on_what_you_use//:apply_fixes", "--", f"--workspace={self._workspace}"]
-        if logging.getLogger().isEnabledFor(logging.DEBUG):
-            cmd.append("--verbose")
-        if extra_args:
-            cmd.extend(extra_args)
-        self._run_cmd(cmd=cmd, check=True)
+        verbosity = ["--verbose"] if logging.getLogger().isEnabledFor(logging.DEBUG) else []
+        cmd_extra_args = extra_args if extra_args else []
+
+        self._run_cmd(
+            cmd=[
+                "bazel",
+                "run",
+                "--nolegacy_external_runfiles",
+                "@depend_on_what_you_use//:apply_fixes",
+                "--",
+                f"--workspace={self._workspace}",
+                *verbosity,
+                *cmd_extra_args,
+            ],
+            check=True,
+        )
 
     def _get_target_attribute(self, target: str, attribute: str) -> Set["str"]:
         """
@@ -114,10 +133,10 @@ def _make_unexpected_output_error(expected: str, output: str) -> Error:
 
     @staticmethod
     def _make_unexpected_deps_error(
-        expected_deps: Set[str] = None,
-        expected_implementation_deps: Set[str] = None,
-        actual_deps: Set[str] = None,
-        actual_implementation_deps: Set[str] = None,
+        expected_deps: Optional[Set[str]] = None,
+        expected_implementation_deps: Optional[Set[str]] = None,
+        actual_deps: Optional[Set[str]] = None,
+        actual_implementation_deps: Optional[Set[str]] = None,
     ) -> Error:
         message = "Unexpected dependencies.\n"
         if expected_deps:

test/apply_fixes/tool_cli/test_use_custom_output_base.py

@@ -15,7 +15,7 @@ def execute_test_logic(self) -> Result:
             self._run_automatic_fix(extra_args=["--fix-unused", f"--bazel-bin={output_base}"])
 
             target_deps = self._get_target_attribute(target=self.test_target, attribute="deps")
-            if (expected := set()) != target_deps:
+            if (expected := set()) != target_deps:  # type: ignore[var-annotated]
                 return self._make_unexpected_deps_error(expected_deps=expected, actual_deps=target_deps)
             else:
                 return Success()

test/apply_fixes/tool_cli/test_utilize_bazel_info.py

@@ -12,7 +12,7 @@ def execute_test_logic(self) -> Result:
         self._run_automatic_fix(extra_args=["--fix-unused", "--use-bazel-info"])
 
         target_deps = self._get_target_attribute(target=self.test_target, attribute="deps")
-        if (expected := set()) != target_deps:
+        if (expected := set()) != target_deps:  # type: ignore[var-annotated]
             return self._make_unexpected_deps_error(expected_deps=expected, actual_deps=target_deps)
         else:
             return Success()

test/apply_fixes/tool_cli/test_utilize_bazel_info_with_custom_compilation_mode.py

@@ -12,7 +12,7 @@ def execute_test_logic(self) -> Result:
         self._run_automatic_fix(extra_args=["--fix-unused", "--use-bazel-info=opt"])
 
         target_deps = self._get_target_attribute(target=self.test_target, attribute="deps")
-        if (expected := set()) != target_deps:
+        if (expected := set()) != target_deps:  # type: ignore[var-annotated]
             return self._make_unexpected_deps_error(expected_deps=expected, actual_deps=target_deps)
         else:
             return Success()

test/aspect/BUILD

@@ -1,9 +1,31 @@
-load("@rules_python//python:defs.bzl", "py_test")
+load("@rules_python//python:defs.bzl", "py_library", "py_test")
 
 py_test(
-    name = "execute_tests_utest",
+    name = "result_test",
     srcs = [
-        "execute_tests_impl.py",
-        "execute_tests_utest.py",
+        "result.py",
+        "result_test.py",
     ],
 )
+
+py_test(
+    name = "test_case_test",
+    srcs = [
+        "test_case.py",
+        "test_case_test.py",
+    ],
+)
+
+py_test(
+    name = "version_test",
+    srcs = [
+        "version.py",
+        "version_test.py",
+    ],
+)
+
+# Helper target to enable mypy
+py_library(
+    name = "python_files",
+    srcs = glob(["**/*.py"]),
+)

test/aspect/alias/test_invalid_dependency_through_alias.py

@@ -0,0 +1,13 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(
+            success=False,
+            invalid_includes=["File='test/aspect/alias/use_a_and_b.cpp', include='test/aspect/alias/a.h'"],
+        )
+        actual = self._run_dwyu(target="//test/aspect/alias:use_a_transitively", aspect=self.default_aspect)
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/alias/test_unused_dependency_through_alias.py

@@ -0,0 +1,10 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=False, unused_public_deps=["//test/aspect/alias:lib_a"])
+        actual = self._run_dwyu(target="//test/aspect/alias:unused_dependency", aspect=self.default_aspect)
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/alias/test_use_dependency_through_alias.py

@@ -0,0 +1,10 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(target="//test/aspect/alias:use_a_directly", aspect=self.default_aspect)
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/aspect.bzl

@@ -1,3 +1,4 @@
 load("//:defs.bzl", "dwyu_aspect_factory")
 
-dwyu_default_aspect = dwyu_aspect_factory()
+dwyu = dwyu_aspect_factory()
+dwyu_impl_deps = dwyu_aspect_factory(use_implementation_deps = True)

test/aspect/complex_includes/test_complex_includes.py

@@ -0,0 +1,18 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        """
+        Working in external repositories has its own challenges due to some paths behaving different compared to
+        working inside the own workspace. Thus, we explicitly test working in an external workspace on top of normal
+        targets from within the main workspace.
+        """
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(
+            target=["//test/aspect/complex_includes:all", "@complex_includes_test_repo//..."],
+            aspect=self.default_aspect,
+        )
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/custom_config/test_custom_ignore_include_paths.py

@@ -0,0 +1,13 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(
+            target="//test/aspect/custom_config:use_multiple_arcane_headers",
+            aspect="//test/aspect/custom_config:aspect.bzl%ignore_include_paths_aspect",
+        )
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/custom_config/test_extra_ignore_include_paths.py

@@ -0,0 +1,13 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(
+            target="//test/aspect/custom_config:use_arcane_header_and_vector",
+            aspect="//test/aspect/custom_config:aspect.bzl%extra_ignore_include_paths_aspect",
+        )
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/custom_config/test_ignore_include_patterns.py

@@ -0,0 +1,13 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(
+            target="//test/aspect/custom_config:use_ignored_patterns",
+            aspect="//test/aspect/custom_config:aspect.bzl%extra_ignore_include_patterns_aspect",
+        )
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/custom_config/test_include_not_covered_by_patterns.py

@@ -0,0 +1,18 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(
+            success=False,
+            invalid_includes=[
+                "File='test/aspect/custom_config/use_not_ignored_header.h', include='example_substring_matching_does_not_work_here.h'"
+            ],
+        )
+        actual = self._run_dwyu(
+            target="//test/aspect/custom_config:use_not_ignored_header",
+            aspect="//test/aspect/custom_config:aspect.bzl%extra_ignore_include_patterns_aspect",
+        )
+
+        return self._check_result(actual=actual, expected=expected)

test/aspect/defines/test_processing_defines.py

@@ -0,0 +1,10 @@
+from result import ExpectedResult, Result
+from test_case import TestCaseBase
+
+
+class TestCase(TestCaseBase):
+    def execute_test_logic(self) -> Result:
+        expected = ExpectedResult(success=True)
+        actual = self._run_dwyu(target="//test/aspect/defines:all", aspect=self.default_aspect)
+
+        return self._check_result(actual=actual, expected=expected)