From 30af51263394e21fa4638883b841edd59d40f317 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 12:25:37 -0700 Subject: [PATCH] fix(parsers/c): codeql build-mode-none, optional-stage success, exclude orchestrators from pytest Three defects surfaced via parsers/c/test_pipeline.py (the C pipeline orchestrator): 1. CodeQL build mode: `codeql database create` for cpp omitted --build-mode=none, so CodeQL defaulted to autobuild and silently degraded (dropping findings) on the no-build / extracted-source repos this pipeline runs on. Pass --build-mode=none. 2. Optional-stage success: overall success ANDed over ALL recorded stages, but the optional stages (CodeQL analysis/filter, reachability filter, context enhancer, exploitable filter) write success=False when they fail or are skipped -- so an optional-stage failure forced a spurious pipeline failure (exit 1). Introduce OPTIONAL_STAGES + a _compute_success() that requires only the non-optional stages. (Cross-parser family: the same all_success conjunction exists in the go/php/ruby/javascript pipelines -- separate units, not widened here.) 3. pytest collection collision: the six parsers//test_pipeline.py files are CLI pipeline ORCHESTRATORS, not pytest tests, but share a basename, so `pytest parsers/` fails with import-file-mismatch. __init__.py does not fix it (their bare local imports -- `from repository_scanner import ...` -- make them un-importable as package modules; adding __init__.py merely moves the collision). Add a root conftest.py that excludes them from collection (collect_ignore_glob), which is correct since they are not tests. The canonical suite is unaffected (pytest.ini already scopes testpaths = tests). Scope: c-specific for #1 (go's codeql create may need its own --build-mode -- separate unit) and #2 (the all_success family is fixed per-parser); #3 is repo-wide (the conftest excludes all six orchestrators). function_extractor / other parsers unchanged. Tests: tests/test_c_pipeline.py -- (1) source-read that the C codeql create cmd passes --build-mode=none; (2) behavioral _compute_success() ignores optional-stage failures but fails on a required-stage failure (loaded via a CONTAINED importlib import that pops the polluting sibling-parser modules in a finally, since c/test_pipeline.py does bare local imports); (3) the root conftest excludes the orchestrators. RED 3 failed (pre-fix) -> GREEN; ruff clean; full suite 179 passed, 63 skipped, 0 failed; `pytest parsers/ --co` now collects cleanly (was import-file-mismatch). Co-Authored-By: Claude Opus 4.7 (1M context) --- libs/openant-core/conftest.py | 4 + libs/openant-core/parsers/c/test_pipeline.py | 32 +++++++- libs/openant-core/tests/test_c_pipeline.py | 78 ++++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 libs/openant-core/conftest.py create mode 100644 libs/openant-core/tests/test_c_pipeline.py diff --git a/libs/openant-core/conftest.py b/libs/openant-core/conftest.py new file mode 100644 index 00000000..f0018cda --- /dev/null +++ b/libs/openant-core/conftest.py @@ -0,0 +1,4 @@ +# The parsers//test_pipeline.py files are CLI pipeline orchestrators (run as scripts), not +# pytest tests. Their shared basename + script-only bare imports make pytest mis-collection fail +# with import-file-mismatch under `pytest parsers/`. Exclude them from collection. +collect_ignore_glob = ["parsers/*/test_pipeline.py"] diff --git a/libs/openant-core/parsers/c/test_pipeline.py b/libs/openant-core/parsers/c/test_pipeline.py index f325a7f6..ad77ddcc 100644 --- a/libs/openant-core/parsers/c/test_pipeline.py +++ b/libs/openant-core/parsers/c/test_pipeline.py @@ -68,6 +68,15 @@ class ProcessingLevel(Enum): EXPLOITABLE = "exploitable" +# Pipeline stages that are OPTIONAL: they may fail or be skipped (e.g. CodeQL not installed, no entry +# points) without the run being a failure. They record success=False on failure, so they must be +# excluded from the overall-success conjunction -- otherwise an optional-stage failure forces exit 1. +OPTIONAL_STAGES = frozenset({ + 'reachability_filter', 'codeql_analysis', 'codeql_filter', + 'context_enhancer', 'exploitable_filter', +}) + + class CPipelineTest: def __init__( self, @@ -376,6 +385,11 @@ def run_codeql_analysis(self) -> bool: codeql_db_path, f'--language={language}', f'--source-root={self.repo_path}', + # These repos carry extracted source with no build system to run, so use the + # build-mode-none extractor: a compiled language (cpp) is indexed without autobuild, + # which would otherwise fail/degrade on no-build/autotools repos and silently drop + # CodeQL findings. + '--build-mode=none', '--overwrite' ] @@ -803,6 +817,19 @@ def apply_exploitable_filter(self) -> bool: self.results['stages']['exploitable_filter'] = result return False + def _compute_success(self) -> bool: + """Overall success = all REQUIRED stages succeeded. + + Optional stages (CodeQL, reachability filter, context enhancer, exploitable filter) write + success=False on failure/skip; ANDing them into overall success made an optional-stage + failure a spurious pipeline failure (exit 1). Exclude them from the conjunction. + """ + return all( + stage.get('success', False) + for name, stage in self.results['stages'].items() + if name not in OPTIONAL_STAGES + ) + def run_full_pipeline(self): """Run the complete pipeline.""" print("=" * 60) @@ -861,10 +888,7 @@ def run_full_pipeline(self): print("PIPELINE SUMMARY") print("=" * 60) - all_success = all( - stage.get('success', False) - for stage in self.results['stages'].values() - ) + all_success = self._compute_success() self.results['success'] = all_success diff --git a/libs/openant-core/tests/test_c_pipeline.py b/libs/openant-core/tests/test_c_pipeline.py new file mode 100644 index 00000000..d568efe5 --- /dev/null +++ b/libs/openant-core/tests/test_c_pipeline.py @@ -0,0 +1,78 @@ +"""Regression tests for three defects in parsers/c/test_pipeline.py. + +1. The C CodeQL `database create` command omits `--build-mode=none`, so cpp + defaults to autobuild and silently degrades (drops findings) on no-build/autotools repos. +2. Overall success ANDed over ALL stages, so an OPTIONAL stage (CodeQL, + reachability, context enhancer, exploitable) failing/skipping forced a spurious pipeline failure. +3. The six same-named parsers//test_pipeline.py orchestrators (CLI + runners, not tests) collide under `pytest parsers/` (import-file-mismatch). __init__.py does NOT fix + it -- their bare local imports make them un-importable as package modules -- so a root conftest + collect_ignore_glob excludes them from collection. + +The _compute_success test contains its import: c/test_pipeline.py does bare local imports +(`from repository_scanner import ...`) that would pollute sys.modules with c's parser modules and +shadow the python parser tests -- so the import is done inside the test under a unique name and the +polluting entries are popped in a finally. +""" +import importlib.util +import sys +from pathlib import Path + +CORE = Path(__file__).resolve().parents[1] # libs/openant-core +C_SRC = CORE / "parsers" / "c" / "test_pipeline.py" + + +# source-read (the codeql cmd cannot run without the CodeQL CLI) +def test_codeql_create_uses_build_mode_none(): + text = C_SRC.read_text() + create_idx = text.index("'codeql', 'database', 'create'") + overwrite_idx = text.index("'--overwrite'", create_idx) + create_cmd = text[create_idx:overwrite_idx] + assert "'--build-mode=none'" in create_cmd, \ + "C codeql `database create` must pass --build-mode=none (no autobuild on no-build cpp repos)" + + +# behavioral, with contained import + sys.modules cleanup +def test_compute_success_ignores_optional_stage_failures(): + cdir = str(CORE / "parsers" / "c") + added = cdir not in sys.path + if added: + sys.path.insert(0, cdir) + before = set(sys.modules) + try: + spec = importlib.util.spec_from_file_location("c_test_pipeline_isolated", str(C_SRC)) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + ct = mod.CPipelineTest.__new__(mod.CPipelineTest) # skip __init__ (no repo I/O needed) + + # required stage ok, optional stages failed -> overall success is True + ct.results = {"stages": { + "c_parser": {"success": True}, + "codeql_analysis": {"success": False}, + "reachability_filter": {"success": False}, + }} + assert ct._compute_success() is True, "optional-stage failures must not fail the pipeline" + + # required stage fails -> overall success is False + ct.results["stages"]["c_parser"]["success"] = False + assert ct._compute_success() is False, "a required stage failure must fail the pipeline" + finally: + if added: + sys.path.remove(cdir) + for m in set(sys.modules) - before: + root = m.split(".")[0] + if root in ("repository_scanner", "function_extractor", "call_graph_builder", + "unit_generator", "c_test_pipeline_isolated"): + sys.modules.pop(m, None) + + +# the parsers//test_pipeline.py orchestrators (CLI runners, NOT +# pytest tests) must be excluded from collection so their shared basename does not collide under +# `pytest parsers/` (import-file-mismatch). __init__.py does not fix it -- the orchestrators' bare +# local imports make them un-importable as package modules -- so we exclude them from collection. +def test_parser_orchestrators_excluded_from_pytest_collection(): + conftest = CORE / "conftest.py" + assert conftest.exists(), "libs/openant-core/conftest.py missing (collect_ignore for orchestrators)" + text = conftest.read_text() + assert "collect_ignore_glob" in text and "parsers/*/test_pipeline.py" in text, \ + "root conftest must collect_ignore_glob the parsers/*/test_pipeline.py orchestrators"