diff --git a/bin/test-polars.sh b/bin/test-polars.sh index f49ab7697a..3bfc16a4dd 100755 --- a/bin/test-polars.sh +++ b/bin/test-polars.sh @@ -12,6 +12,12 @@ set -ex python -m pytest --version # Single source of truth for the polars test file list (CI reuses this script). +# +# COMPLETENESS IS ENFORCED, NOT REMEMBERED: polars is installed in NO other CI lane, so a +# module gated by `pytest.importorskip("polars")` that is missing from this array runs +# NOWHERE — it is collection-skipped everywhere else and silently reports green. +# `graphistry/tests/compute/gfql/test_polars_lane_completeness.py` parses this array and +# fails if any module-level polars-gated test file is absent from it (or listed but gone). POLARS_TEST_FILES=( graphistry/tests/compute/test_polars.py graphistry/tests/compute/gfql/test_engine_polars_hop.py @@ -29,6 +35,20 @@ POLARS_TEST_FILES=( graphistry/tests/compute/gfql/test_polars_rows_entity_groupby.py graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py graphistry/tests/compute/gfql/test_residual_polars_native.py + # module-level `importorskip("polars")` files that previously ran in no lane at all + graphistry/tests/compute/gfql/test_engine_polars_narrow_combine.py + graphistry/tests/compute/gfql/test_engine_polars_semi_key_dedup.py + graphistry/tests/compute/gfql/test_engine_polars_call_modality.py + graphistry/tests/compute/gfql/test_engine_polars_gpu.py + graphistry/tests/compute/gfql/test_rows_table_named_middle.py + graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py + # polars-parametrized cases inside otherwise-pandas modules: these files DO run in the + # pandas lanes, but their polars/polars-gpu parameters are skipped there for want of the + # wheel, so the polars lane is the only place those parameters can execute + graphistry/tests/compute/gfql/index/test_indexed_bindings.py + graphistry/tests/compute/gfql/test_reentry_caller_graph_immutability.py + graphistry/tests/compute/gfql/test_rewrite_param_discard.py + graphistry/tests/compute/test_engine_coercion.py # index tests exercise the seeded-index hook in the polars hop entry (hop.py) — without # them the hook dominates the now-thin file and trips its per-file coverage floor graphistry/tests/compute/gfql/index/test_index.py diff --git a/graphistry/tests/compute/gfql/cypher/test_lowering.py b/graphistry/tests/compute/gfql/cypher/test_lowering.py index 093c727fcc..d6960ee15c 100644 --- a/graphistry/tests/compute/gfql/cypher/test_lowering.py +++ b/graphistry/tests/compute/gfql/cypher/test_lowering.py @@ -15913,7 +15913,7 @@ def dtypes(self) -> Any: assert dict(dtypes) == {} -def test_node_dtypes_for_pushdown_defers_the_conversion() -> None: +def test_node_dtypes_for_pushdown_on_polars_defers_the_conversion() -> None: # Reading dtypes costs a full engine conversion, and only connected-join pushdown asks -- # a path most queries never reach. Computing eagerly charged every string query for a # frame it discarded. Nothing may convert until a lookup actually happens. @@ -15944,7 +15944,7 @@ def spy(*args: Any, **kwargs: Any) -> Any: filter_by_dict._read_node_dtypes = original -def test_node_dtypes_for_pushdown_matches_the_full_conversion() -> None: +def test_node_dtypes_for_pushdown_on_polars_matches_the_full_conversion() -> None: # polars -> pandas is DATA-dependent: an empty probe reports `bool` for a nullable Boolean # while the real conversion yields `object`. The gate must report what the executor sees, # so compare against the full conversion rather than asserting probe dtypes in isolation. diff --git a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py index 55e9a6d367..2c44c25ce5 100644 --- a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py +++ b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py @@ -1110,9 +1110,17 @@ def test_use_policy_sparse_serves_dense_declines( assert decisions[0]["reason"] == expected_reason -def test_explicit_polars_gpu_declines_indexed_helper_and_falls_back( - monkeypatch: pytest.MonkeyPatch, -) -> None: +def _cudf_polars_available() -> bool: + """Mirrors chain.py's own gate: engine='polars-gpu' EXECUTION raises ImportError without + the RAPIDS cudf_polars stack. The helper-declines half needs no GPU at all.""" + import importlib.util + + return importlib.util.find_spec("cudf_polars") is not None + + +def test_explicit_polars_gpu_declines_indexed_helper() -> None: + """CPU-only half: the indexed helper must decline for Engine.POLARS_GPU. Split out of the + fall-back test so it runs wherever polars does, instead of being lost behind a GPU gate.""" import graphistry.compute.gfql.index.bindings as indexed_bindings g = _graph("polars-gpu") @@ -1124,6 +1132,16 @@ def test_explicit_polars_gpu_declines_indexed_helper_and_falls_back( assert indexed_bindings.try_indexed_connected_bindings_state( g, ops, engine=Engine.POLARS_GPU ) is None + + +@pytest.mark.skipif( + not _cudf_polars_available(), + reason="engine='polars-gpu' execution requires the RAPIDS cudf_polars stack", +) +def test_explicit_polars_gpu_declines_indexed_helper_and_falls_back( + monkeypatch: pytest.MonkeyPatch, +) -> None: + g = _graph("polars-gpu") _assert_parity( g, CONNECTED_QUERY, diff --git a/graphistry/tests/compute/gfql/test_polars_lane_completeness.py b/graphistry/tests/compute/gfql/test_polars_lane_completeness.py new file mode 100644 index 0000000000..0113099daa --- /dev/null +++ b/graphistry/tests/compute/gfql/test_polars_lane_completeness.py @@ -0,0 +1,196 @@ +"""Guard: a polars-gated test may not be invisible to CI. + +WHY THIS EXISTS (measured, not hypothetical). polars is installed in exactly ONE CI lane +-- ``test-polars``, driven by ``bin/test-polars.sh`` -- and that lane runs an explicit file +list. Every other lane collection-skips a module-level ``pytest.importorskip("polars")`` and +per-test ``polars``/``polars-gpu`` parameters for want of the wheel. So a polars-gated test +that the lane's file list omits executes NOWHERE, while both lanes report green. + +Measured on GitHub Actions run 30311675717: 280 tests ran locally with polars installed and +appeared in no CI job log at all, including whole modules named ``test_engine_polars_*``. +Two real defects were sitting in that blind spot (a polars-gpu test that fails without the +RAPIDS stack, and a pandas/polars ``toUpper`` divergence). + +The rule enforced here: every test module that mentions polars is EITHER in the lane's file +list OR carries a written reason for being out of it. Silence is not a valid answer. +""" +from __future__ import annotations + +import ast +import re +from pathlib import Path +from typing import Dict, List, Set, Union + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[4] +LANE_SCRIPT = REPO_ROOT / "bin" / "test-polars.sh" +TEST_ROOTS = ("graphistry/tests", "tests") + +# Second lane phase: this module runs under ``-k polars``, so only node ids containing +# "polars" execute. Kept out of the main array on purpose (runtime), but its polars-gated +# tests still have to be selectable -- enforced by +# ``test_lowering_polars_gated_tests_are_named_so_the_k_filter_selects_them``. +K_FILTERED_MODULE = "graphistry/tests/compute/gfql/cypher/test_lowering.py" + +# This guard itself: it is pure static analysis over source text, needs no polars wheel, and +# only matches its own scanners because it QUOTES the gate it looks for. +SELF = "graphistry/tests/compute/gfql/test_polars_lane_completeness.py" + +# Modules that mention polars but are NOT polars-gated, each with the reason. A new entry +# here is a deliberate, reviewable statement -- which is the point of the guard. +NOT_POLARS_GATED: Dict[str, str] = { + "graphistry/tests/compute/gfql/cypher/test_row_pushdown.py": ( + "single engine-agnostic to_pandas() branch; every test runs on the pandas lane" + ), + "graphistry/tests/test_compute_filter_by_dict.py": ( + "pandas oracle only; the sentence naming polars is a docstring cross-reference" + ), + "graphistry/tests/compute/gfql/index/test_index_gpu_edge_match.py": ( + "cudf/GPU-gated (module-level importorskip('cudf') + skipif no GPU), not polars-gated; " + "belongs to the separate GPU-lane gap, and the polars CPU lane could not run it" + ), +} + +_POLARS_IMPORTORSKIP = re.compile(r"""importorskip\(\s*["']polars["']""") + + +def _lane_file_list() -> List[str]: + """Parse ``POLARS_TEST_FILES`` out of bin/test-polars.sh. + + Deliberately strict: if the array cannot be found the guard fails rather than passing + vacuously, so restructuring the script cannot silently disable this check. + """ + text = LANE_SCRIPT.read_text(encoding="utf-8") + match = re.search(r"^POLARS_TEST_FILES=\((.*?)^\)", text, re.MULTILINE | re.DOTALL) + assert match is not None, ( + f"{LANE_SCRIPT} no longer declares a POLARS_TEST_FILES=( ... ) array; this guard " + "parses it as the lane's single source of truth" + ) + files: List[str] = [] + for raw in match.group(1).splitlines(): + line = raw.split("#", 1)[0].strip() + if line: + files.append(line) + assert files, "POLARS_TEST_FILES parsed as empty" + return files + + +FuncDef = Union[ast.FunctionDef, ast.AsyncFunctionDef] + + +def _node_source(source_lines: List[str], node: Union[ast.stmt, ast.expr]) -> str: + """py3.8-safe source slice for an AST node (``ast.unparse`` is 3.9+).""" + start: int = node.lineno + end: int = node.end_lineno if node.end_lineno is not None else start + return "\n".join(source_lines[max(start - 1, 0):end]) + + +def _selected_by_k_polars(source_lines: List[str], node: FuncDef) -> bool: + """True when ``-k polars`` can select the node: either the function name carries it, or a + decorator does (a ``parametrize`` over an engine id containing 'polars' puts it in the + node id, which is what ``-k`` matches against).""" + if "polars" in node.name: + return True + return any( + "polars" in _node_source(source_lines, dec) for dec in node.decorator_list + ) + + +def _test_modules_mentioning_polars() -> Set[str]: + found: Set[str] = set() + for root in TEST_ROOTS: + base = REPO_ROOT / root + if not base.is_dir(): + continue + for path in base.rglob("test_*.py"): + if "polars" in path.read_text(encoding="utf-8").lower(): + found.add(path.relative_to(REPO_ROOT).as_posix()) + return found + + +def test_lane_file_list_paths_all_exist() -> None: + """A renamed or deleted test file must not silently shrink the lane.""" + missing = [rel for rel in _lane_file_list() if not (REPO_ROOT / rel).is_file()] + assert missing == [], f"bin/test-polars.sh lists files that do not exist: {missing}" + + +def test_every_polars_mentioning_test_module_is_in_the_lane_or_justified() -> None: + """The completeness rule. New polars test file => lane entry or written exemption.""" + lane = set(_lane_file_list()) | {K_FILTERED_MODULE, SELF} + unclassified = sorted( + _test_modules_mentioning_polars() - lane - set(NOT_POLARS_GATED) + ) + assert unclassified == [], ( + "these test modules mention polars but run in NO CI lane (polars is installed only " + "in test-polars, which runs an explicit file list): " + f"{unclassified}. Add each to POLARS_TEST_FILES in bin/test-polars.sh, or to " + "NOT_POLARS_GATED here with the reason it needs no polars lane." + ) + + +def test_not_polars_gated_entries_are_not_stale() -> None: + """An exemption for a file that is gone, or that the lane now runs, must be removed.""" + lane = set(_lane_file_list()) | {K_FILTERED_MODULE} + gone = sorted(rel for rel in NOT_POLARS_GATED if not (REPO_ROOT / rel).is_file()) + assert gone == [], f"NOT_POLARS_GATED names files that no longer exist: {gone}" + contradictory = sorted(set(NOT_POLARS_GATED) & lane) + assert contradictory == [], ( + f"NOT_POLARS_GATED claims these need no polars lane, but the lane runs them: " + f"{contradictory}" + ) + + +def test_no_module_level_polars_gate_outside_the_lane() -> None: + """The sharpest form of the hole: a module-level importorskip skips the WHOLE file + everywhere else, so omission from the lane costs every test in it at once.""" + lane = set(_lane_file_list()) | {K_FILTERED_MODULE, SELF} + offenders: List[str] = [] + for root in TEST_ROOTS: + base = REPO_ROOT / root + if not base.is_dir(): + continue + for path in base.rglob("test_*.py"): + rel = path.relative_to(REPO_ROOT).as_posix() + if rel in lane: + continue + source = path.read_text(encoding="utf-8") + source_lines = source.splitlines() + for node in ast.parse(source).body: # module level only + if _POLARS_IMPORTORSKIP.search(_node_source(source_lines, node)): + offenders.append(rel) + break + assert offenders == [], ( + "module-level pytest.importorskip('polars') outside the polars lane -- every test in " + f"these files is collection-skipped in every CI lane: {offenders}" + ) + + +def test_lowering_polars_gated_tests_are_named_so_the_k_filter_selects_them() -> None: + """``test_lowering.py`` runs under ``-k polars``. A test that calls + ``importorskip("polars")`` but has no 'polars' in its function name is deselected in the + polars lane and skipped in the pandas lanes -- i.e. it runs nowhere.""" + path = REPO_ROOT / K_FILTERED_MODULE + source = path.read_text(encoding="utf-8") + source_lines = source.splitlines() + offenders = [ + node.name + for node in ast.parse(source).body + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name.startswith("test_") + and not _selected_by_k_polars(source_lines, node) + and _POLARS_IMPORTORSKIP.search(_node_source(source_lines, node)) + ] + assert offenders == [], ( + f"{K_FILTERED_MODULE} is run with `-k polars`; these polars-gated tests are not " + f"selected by that filter and therefore run in no lane: {offenders}. Put 'polars' in " + "the test name (or parametrize it over a 'polars' engine id)." + ) + + +@pytest.mark.parametrize("rel", sorted(NOT_POLARS_GATED)) +def test_exemption_reasons_are_substantive(rel: str) -> None: + """An empty or placeholder reason would re-open the hole under the guard's own cover.""" + reason = NOT_POLARS_GATED[rel] + assert len(reason) >= 40, f"exemption for {rel} needs a real reason, got {reason!r}" diff --git a/graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py b/graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py index 345f082d4a..536aeeeb1d 100644 --- a/graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py +++ b/graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py @@ -466,8 +466,50 @@ def _trick_matrix(): ] -@pytest.mark.parametrize("label,query,pinned,mirror", _trick_matrix(), - ids=[t[0] for t in _trick_matrix()]) +def _pandas_str_accessor_is_full_case_mapping() -> bool: + """pandas<3 stores text as object and delegates .str.upper()/.lower() to Python, which + applies FULL Unicode case mapping ('straße' -> 'STRASSE', 'İ' -> 'i̇'). + pandas>=3 backs the default `str` dtype with Arrow, whose utf8_upper/utf8_lower are + SIMPLE per-codepoint mappings ('straße' -> 'STRAẞE', 'İ' -> 'i').""" + upper = pd.Series(["straße"]).str.upper().iloc[0] + lower = pd.Series(["İ"]).str.lower().iloc[0] + return bool(upper == "STRASSE" and lower == "i̇") + + +# GFQL's pandas toLower/toUpper delegate to that accessor, so under pandas>=3 they return +# SIMPLE mappings while the polars engine (Rust to_uppercase/to_lowercase) and Cypher itself +# (neo4j toUpper == Java String.toUpperCase) return FULL ones. That is a silent cross-engine +# divergence, not a decline: pandas answers [9] where polars answers [8, 9]. Tracked in +# https://github.com/graphistry/pygraphistry/issues/1802 — the hand pins below stay at the +# Cypher-correct values, so `strict=True` turns the fix into an XPASS that retires this mark. +_SIMPLE_CASE_MAPPING_XFAIL = pytest.mark.xfail( + not _pandas_str_accessor_is_full_case_mapping(), + reason=( + "pandas>=3 Arrow-backed str dtype does SIMPLE case mapping; GFQL pandas toLower/" + "toUpper inherit it and diverge from the polars engine and from Cypher (#1802)" + ), + strict=True, +) + +_SIMPLE_CASE_MAPPING_LABELS = frozenset({"toupper-eq-ss-fold", "tolower-turkish-dotted-i"}) + + +def _trick_params(): + return [ + pytest.param( + *case, + id=case[0], + marks=( + [_SIMPLE_CASE_MAPPING_XFAIL] + if case[0] in _SIMPLE_CASE_MAPPING_LABELS + else [] + ), + ) + for case in _trick_matrix() + ] + + +@pytest.mark.parametrize("label,query,pinned,mirror", _trick_params()) def test_case_regex_unicode_trick_matrix(label, query, pinned, mirror): g = _panel_graph() nd = _panel_nodes()