From 0f7cd5b7ac26c5c704e3c2715a7bd806b608dc9b Mon Sep 17 00:00:00 2001 From: Niko Date: Mon, 12 Oct 2026 23:04:58 +0200 Subject: [PATCH] fix(conventions): skip library test fixtures in the export checks The duplicate-export and silent-stub rules treated every APS5_VABI definition under core/libs/prx/ as a library export, including mocks and callbacks in a library's tests/ directory. Those files are built only into separate test executables, never into the library, so they are not exports. PR #3533 failed CI because a sceKernelGetProcessTime mock in libSceAudioOut/tests was reported as a duplicate of libkernel's export. An sce* fixture with a stub-shaped body was also reported as a silent stub, and a real export that shares a name with a fixture in another library's tests/ was reported as a duplicate. Skip files under a tests directory on both sides of the check, with the same test code() already uses for title-specific and progress.py uses for its function counts. Co-Authored-By: Claude Opus 5.5 --- tools/check_conventions.py | 4 +- .../tests/test_check_conventions_comments.py | 52 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/tools/check_conventions.py b/tools/check_conventions.py index 34b639f16..44bb8eca4 100644 --- a/tools/check_conventions.py +++ b/tools/check_conventions.py @@ -109,7 +109,7 @@ class Check: if core and TITLE.search(text): self.report("title-specific", path, number, TITLE.search(text).group(0)) match = EXPORT.search(bare) - if match and path.startswith("core/libs/prx/") and not match.group(1).endswith(("_nid_no_patch", "_nid_no_patch_cut")): + if match and core and path.startswith("core/libs/prx/") and not match.group(1).endswith(("_nid_no_patch", "_nid_no_patch_cut")): source = source or self.show(path).splitlines() body = function_body(source, number - 1) if body is None: @@ -134,7 +134,7 @@ class Check: _, path, number, text = line.split(":", 3) name = EXPORT.search(text).group(1) own, own_number = exports[name] - if library(path) != library(own) and function_body(self.show(path).splitlines(), int(number) - 1) is not None: + if library(path) != library(own) and "tests" not in PurePosixPath(path).parts and function_body(self.show(path).splitlines(), int(number) - 1) is not None: self.report("duplicate-export", own, own_number, f"{name} is also in {library(path)}") def build(self): diff --git a/tools/tests/test_check_conventions_comments.py b/tools/tests/test_check_conventions_comments.py index 4d3e83c0b..a00b924fd 100644 --- a/tools/tests/test_check_conventions_comments.py +++ b/tools/tests/test_check_conventions_comments.py @@ -245,6 +245,58 @@ class SilentStubSectionTests(unittest.TestCase): self.assertNotIn("[debt-file]", result.stdout + result.stderr) +class TestFixtureExportTests(unittest.TestCase): + def check(self, path, text): + checker = Path(__file__).resolve().parents[1] / "check_conventions.py" + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + + def git(*args): + return subprocess.run( + ["git", "-c", f"safe.directory={root}", "-c", "user.name=Conventions Test", + "-c", "user.email=test@example.com", "-c", "commit.gpgsign=false", *args], + cwd=root, capture_output=True, text=True, check=True, + ) + + def write(relative, content): + target = root / relative + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + + git("init", "-q") + write("core/libs/prx/libkernel/src/Time.cpp", "std::uint64_t APS5_VABI sceKernelGetProcessTime() { return now(); }\n") + write("core/libs/prx/libSceNgs2/tests/Render.cpp", "static int APS5_VABI Process(int value) { return value; }\n") + git("add", ".") + git("commit", "-qm", "feat: add libraries") + write(path, text) + git("add", ".") + git("commit", "-qm", "feat: add source") + env = {key: value for key, value in os.environ.items() + if key not in ("GITHUB_EVENT_PATH", "GITHUB_STEP_SUMMARY", "GITHUB_ACTIONS")} + env.update(GIT_CONFIG_COUNT="1", GIT_CONFIG_KEY_0="safe.directory", GIT_CONFIG_VALUE_0=str(root)) + return subprocess.run( + [sys.executable, "-I", str(checker), "--base", "HEAD~1"], + cwd=root, capture_output=True, text=True, env=env, timeout=20, + ) + + def test_mock_in_tests_directory_is_not_an_export(self): + result = self.check("core/libs/prx/libSceAudioOut/tests/Shutdown.cpp", + 'extern "C" std::uint64_t APS5_VABI sceKernelGetProcessTime() { return 0; }\n') + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("All conventions checks passed", result.stdout) + + def test_export_named_like_a_test_fixture_is_not_a_duplicate(self): + result = self.check("core/libs/prx/libSceAjm/src/Port.cpp", "int APS5_VABI Process(int value) { return value + 1; }\n") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("All conventions checks passed", result.stdout) + + def test_export_in_another_library_is_still_a_duplicate(self): + result = self.check("core/libs/prx/libSceAudioOut/src/Clock.cpp", "std::uint64_t APS5_VABI sceKernelGetProcessTime() { return now(); }\n") + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("core/libs/prx/libSceAudioOut/src/Clock.cpp:1: error: [duplicate-export]", result.stdout) + self.assertIn("sceKernelGetProcessTime is also in libkernel", result.stdout) + + class PullRequestTemplateTests(unittest.TestCase): def findings(self, line): check = Check.__new__(Check)