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 <noreply@anthropic.com>
This commit is contained in:
parent
fdefd76609
commit
0f7cd5b7ac
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue