Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions bin/test-polars.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions graphistry/tests/compute/gfql/cypher/test_lowering.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
24 changes: 21 additions & 3 deletions graphistry/tests/compute/gfql/index/test_indexed_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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,
Expand Down
196 changes: 196 additions & 0 deletions graphistry/tests/compute/gfql/test_polars_lane_completeness.py
Original file line number Diff line number Diff line change
@@ -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}"
46 changes: 44 additions & 2 deletions graphistry/tests/compute/gfql/test_viz_pipeline_conformance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading