Skip to content

Commit 154de45

Browse files
committed
Batch parity fixtures: 1 INSERT + 1 SELECT for the whole 510-pair suite
Previous shape ran 102 INSERTs (one per case) plus 510 SELECTs (one per parametrised test) — ~612 Snowflake round-trips. With xdist on 4 CPUs the parity job took ~2m24s in CI, dominated by per-query latency, not compute. Two batches: - `loaded_cases` does one multi-row INSERT for all 102 cases. - `parity_results` runs a single UNION-ALL'd SELECT that evaluates every translated segment against every case's identity row, returns a `(case_idx, seg_key) -> bool | None` dict. Per-test invocation is a dict lookup; no Snowflake call in the parametrised body. Local bench: 31s wall time for 510 passing tests, serial. Down from ~2m24s. Drops `-n auto` from the CI workflow — the bottleneck is one Snowflake query, not pytest concurrency, so xdist would just multiply the mega-query setup across workers without speedup. beep boop
1 parent 66a9698 commit 154de45

3 files changed

Lines changed: 120 additions & 70 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,4 @@ jobs:
3232
run: |
3333
umask 077
3434
printf '%s' "${{ secrets.SNOWFLAKE_PRIVATE_KEY }}" > /tmp/snowflake_pk.p8
35-
- run: make test opts="-n auto"
35+
- run: make test

tests/conftest.py

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,27 @@
11
"""Pytest fixtures.
22
33
Unit tests run anywhere; the parity suite needs a Snowflake account. The
4-
parity fixture skips automatically if `SNOWFLAKE_ACCOUNT` isn't set, so
4+
parity fixtures skip automatically if `SNOWFLAKE_ACCOUNT` isn't set, so
55
running `pytest` with no env vars only runs the unit tests.
66
77
Each parity-test session creates a per-run transient `IDENTITIES_PARITY_<uuid>`
88
table so concurrent CI runs don't step on each other. Schema mirrors
99
`SnowflakeDialect.SCHEMA_DDL`: 4 typed columns + a `traits` VARIANT.
1010
Table is dropped on teardown.
11+
12+
The parity fixtures are batched to keep Snowflake round-trips down:
13+
14+
- All test-case identities go into the scratch table in a single
15+
`INSERT INTO ... VALUES (...), (...), ...` statement.
16+
- All test-case (segment, identity) pairs are evaluated in a single
17+
`SELECT ... UNION ALL ...` mega-query, returning a `(case_idx, seg_key)
18+
-> bool` dict. Per-test parametrised tests then do an in-memory dict
19+
lookup. Two Snowflake round-trips for the whole suite.
1120
"""
1221

1322
from __future__ import annotations
1423

24+
import copy
1525
import json
1626
import os
1727
import uuid
@@ -20,6 +30,9 @@
2030
from typing import Any
2131

2232
import pytest
33+
from flag_engine.context.types import EnvironmentContext
34+
35+
from flagsmith_sql_flag_engine import TranslateContext, translate_segment
2336

2437
REPO_ROOT = Path(__file__).resolve().parents[1]
2538
ENGINE_TEST_DATA = REPO_ROOT / "engine-test-data" / "test_cases"
@@ -100,3 +113,94 @@ def parity_env_key(case_idx: int, original: str) -> str:
100113
cases (or cases that share an `environment.key` in the source data)
101114
don't see each other's identities."""
102115
return f"parity-{case_idx}-{original}"
116+
117+
118+
def _q(s: str) -> str:
119+
return s.replace("'", "''")
120+
121+
122+
@pytest.fixture(scope="session")
123+
def loaded_cases(snowflake_session: Any, parity_table: str) -> Iterator[list[dict]]:
124+
"""Load every test case's identity into the scratch IDENTITIES table
125+
in a single multi-row INSERT. Returns the list of cases (with
126+
overridden `environment.key` so engine and SQL agree on what env to
127+
filter by). One Snowflake round-trip for all 102 cases.
128+
"""
129+
cases = load_test_cases()
130+
if not cases:
131+
pytest.skip("engine-test-data submodule not initialised")
132+
overridden: list[dict] = []
133+
selects: list[str] = []
134+
for i, case in enumerate(cases):
135+
case = copy.deepcopy(case)
136+
ctx = case["context"]
137+
env = ctx.get("environment") or {}
138+
env["key"] = parity_env_key(i, env.get("key", ""))
139+
env_id = _q(env["key"])
140+
141+
ident = ctx.get("identity") or {}
142+
identifier = _q(ident.get("identifier") or "")
143+
identity_key = _q(ident.get("key") or "")
144+
identity_id = i + 1
145+
traits = ident.get("traits") or {}
146+
if traits:
147+
traits_lit = f"PARSE_JSON('{_q(json.dumps(traits))}')"
148+
else:
149+
traits_lit = "NULL"
150+
selects.append(
151+
f"SELECT '{env_id}', {identity_id}, '{identifier}', '{identity_key}', {traits_lit}"
152+
)
153+
overridden.append(case)
154+
155+
snowflake_session.sql(
156+
f"INSERT INTO {parity_table} (environment_id, id, identifier, identity_key, traits) "
157+
+ "\nUNION ALL\n".join(selects)
158+
).collect()
159+
yield overridden
160+
161+
162+
@pytest.fixture(scope="session")
163+
def parity_results(
164+
snowflake_session: Any,
165+
parity_table: str,
166+
loaded_cases: list[dict],
167+
) -> dict[tuple[int, str], bool | None]:
168+
"""Run every (case, segment) pair's translated SQL in one mega-query
169+
and return a `(case_idx, seg_key) -> bool | None` dict. `None` means
170+
the segment uses an operator the translator can't handle (test will
171+
pytest.skip on that key).
172+
173+
One Snowflake round-trip for all 510 pairs.
174+
"""
175+
pairs: list[tuple[int, str, str | None, str]] = []
176+
select_clauses: list[str] = []
177+
for case_idx, case in enumerate(loaded_cases):
178+
eval_ctx = case["context"]
179+
env: EnvironmentContext = eval_ctx["environment"]
180+
for seg_key, segment in (eval_ctx.get("segments") or {}).items():
181+
tr_ctx = TranslateContext(environment=env)
182+
sql = translate_segment(segment, tr_ctx)
183+
pairs.append((case_idx, seg_key, sql, env["key"]))
184+
185+
for i, (_case_idx, _seg_key, sql, env_key) in enumerate(pairs):
186+
if sql is None:
187+
continue
188+
env_lit = _q(env_key)
189+
select_clauses.append(
190+
f"SELECT {i} AS pair_id, "
191+
f"EXISTS (SELECT 1 FROM {parity_table} i "
192+
f"WHERE i.environment_id = '{env_lit}' AND ({sql})) AS m"
193+
)
194+
195+
results: dict[tuple[int, str], bool | None] = {}
196+
if select_clauses:
197+
rows = snowflake_session.sql("\nUNION ALL\n".join(select_clauses)).collect()
198+
for row in rows:
199+
i = int(row["PAIR_ID"])
200+
case_idx, seg_key, _sql, _env = pairs[i]
201+
results[(case_idx, seg_key)] = bool(row["M"])
202+
# Fill in untranslatable pairs as None so test parametrisation can skip.
203+
for case_idx, seg_key, sql, _env in pairs:
204+
if sql is None:
205+
results[(case_idx, seg_key)] = None
206+
return results

tests/test_engine.py

Lines changed: 14 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,22 @@
11
"""Parity suite: every engine-test-data case translated and run against
22
Snowflake, compared to flag_engine.is_context_in_segment.
33
4-
Skipped if SNOWFLAKE_* env vars aren't set, so `pytest` without creds
5-
runs the unit suite only. CI sets the creds via secrets and runs this
6-
suite as a separate job.
4+
Skipped if SNOWFLAKE_* env vars aren't set. CI sets the creds via secrets
5+
and runs this suite as part of `make test`.
6+
7+
The Snowflake round-trips happen in `conftest.py`'s session-scoped
8+
fixtures: one INSERT for all 102 cases, one SELECT mega-query for all
9+
510 pair evaluations. The per-test function below is just a dict lookup
10+
against the pre-computed `parity_results` and a comparison against the
11+
engine's in-memory result.
712
"""
813

914
from __future__ import annotations
1015

11-
import copy
12-
import json
13-
from collections.abc import Iterator
14-
from typing import Any
15-
1616
import pytest
17-
from flag_engine.context.types import EnvironmentContext
1817
from flag_engine.segments.evaluator import is_context_in_segment
1918

20-
from flagsmith_sql_flag_engine import TranslateContext, translate_segment
21-
from tests.conftest import load_test_cases, parity_env_key
22-
23-
24-
@pytest.fixture(scope="session")
25-
def loaded_cases(snowflake_session: Any, parity_table: str) -> Iterator[list[dict]]:
26-
"""Load every test case's identity into the scratch IDENTITIES table
27-
with traits as a VARIANT (PARSE_JSON-encoded). Returns the list of
28-
cases (with overridden `environment.key` so engine and SQL agree on
29-
what env to filter by).
30-
"""
31-
cases = load_test_cases()
32-
if not cases:
33-
pytest.skip("engine-test-data submodule not initialised")
34-
out: list[dict] = []
35-
for i, case in enumerate(cases):
36-
case = copy.deepcopy(case)
37-
ctx = case["context"]
38-
env = ctx.get("environment") or {}
39-
original_key = env.get("key", "")
40-
env["key"] = parity_env_key(i, original_key)
41-
env_id = env["key"].replace("'", "''")
42-
43-
ident = ctx.get("identity") or {}
44-
identifier = (ident.get("identifier") or "").replace("'", "''")
45-
identity_key = (ident.get("key") or "").replace("'", "''")
46-
identity_id = i + 1
47-
traits = ident.get("traits") or {}
48-
if traits:
49-
traits_json = json.dumps(traits).replace("'", "''")
50-
traits_lit = f"PARSE_JSON('{traits_json}')"
51-
else:
52-
traits_lit = "NULL"
53-
snowflake_session.sql(
54-
f"""
55-
INSERT INTO {parity_table} (environment_id, id, identifier, identity_key, traits)
56-
SELECT '{env_id}', {identity_id}, '{identifier}', '{identity_key}', {traits_lit}
57-
"""
58-
).collect()
59-
out.append(case)
60-
yield out
19+
from tests.conftest import load_test_cases
6120

6221

6322
def _all_case_segments() -> list[tuple[int, str]]:
@@ -79,31 +38,18 @@ def _all_case_segments() -> list[tuple[int, str]]:
7938
def test_segment_matches_engine(
8039
case_idx: int,
8140
seg_key: str,
82-
snowflake_session: Any,
8341
loaded_cases: list[dict],
84-
parity_table: str,
42+
parity_results: dict[tuple[int, str], bool | None],
8543
) -> None:
8644
case = loaded_cases[case_idx]
8745
eval_ctx = case["context"]
88-
env: EnvironmentContext = eval_ctx["environment"]
8946
segment = eval_ctx["segments"][seg_key]
9047

91-
engine_match = is_context_in_segment(eval_ctx, segment)
92-
93-
tr_ctx = TranslateContext(environment=env)
94-
sql = translate_segment(segment, tr_ctx)
95-
if sql is None:
48+
sql_match = parity_results[(case_idx, seg_key)]
49+
if sql_match is None:
9650
pytest.skip(f"segment uses untranslatable operator (case {case_idx} seg {seg_key})")
97-
env_id_lit = env["key"].replace("'", "''")
98-
rows = snowflake_session.sql(
99-
f"""
100-
SELECT EXISTS(
101-
SELECT 1 FROM {parity_table} i
102-
WHERE i.environment_id = '{env_id_lit}' AND ({sql})
103-
) AS m
104-
"""
105-
).collect()
106-
sql_match = bool(rows[0]["M"])
51+
52+
engine_match = is_context_in_segment(eval_ctx, segment)
10753
assert engine_match == sql_match, (
10854
f"engine={engine_match} sql={sql_match} for case={case_idx} seg={seg_key}"
10955
)

0 commit comments

Comments
 (0)