DEV-1786: PR #286 review G4 — test hardening (crossing discrimination, vacuous passes, brittle asserts)

tests/_dev1746_fixtures.py

@@ -63,54 +63,56 @@
 def seed_dev1746_sqlite(db_path: str) -> None:
     """Create + seed the DEV-1746 SQLite corpus at ``db_path``."""
     con = sqlite3.connect(db_path)
-    con.executescript(
-        """
-        CREATE TABLE regions (
-            id INTEGER PRIMARY KEY,
-            name TEXT,
-            population REAL
-        );
-        CREATE TABLE customers (
-            id INTEGER PRIMARY KEY,
-            region_id INTEGER,
-            tier TEXT,
-            spend REAL
-        );
-        CREATE TABLE orders (
-            id INTEGER PRIMARY KEY,
-            customer_id INTEGER,
-            status TEXT,
-            created_at TEXT,
-            amount REAL
-        );
-        """
-    )
-    con.executemany(
-        "INSERT INTO regions VALUES (?,?,?)",
-        # Region 2's name is NULL — the joined nullable grain member.
-        [(1, "West", 100.0), (2, None, 200.0)],
-    )
-    con.executemany(
-        "INSERT INTO customers VALUES (?,?,?,?)",
-        # Customer 101's tier is NULL — the target-side nullable grain member.
-        [
-            (100, 1, "gold", 1000.0),
-            (101, 2, None, 250.0),
-            (102, 2, None, 75.0),
-        ],
-    )
-    con.executemany(
-        "INSERT INTO orders VALUES (?,?,?,?,?)",
-        [
-            (1, 100, "paid", "2024-01-15", PAID_JAN),
-            (2, 100, "paid", "2024-02-15", PAID_FEB),
-            # The NULL-status group — two months, so a 90-day window spans both.
-            (3, 101, None, "2024-01-20", NULL_STATUS_JAN),
-            (4, 101, None, "2024-02-20", NULL_STATUS_FEB),
-        ],
-    )
-    con.commit()
-    con.close()
+    try:
+        con.executescript(
+            """
+            CREATE TABLE regions (
+                id INTEGER PRIMARY KEY,
+                name TEXT,
+                population REAL
+            );
+            CREATE TABLE customers (
+                id INTEGER PRIMARY KEY,
+                region_id INTEGER,
+                tier TEXT,
+                spend REAL
+            );
+            CREATE TABLE orders (
+                id INTEGER PRIMARY KEY,
+                customer_id INTEGER,
+                status TEXT,
+                created_at TEXT,
+                amount REAL
+            );
+            """
+        )
+        con.executemany(
+            "INSERT INTO regions VALUES (?,?,?)",
+            # Region 2's name is NULL — the joined nullable grain member.
+            [(1, "West", 100.0), (2, None, 200.0)],
+        )
+        con.executemany(
+            "INSERT INTO customers VALUES (?,?,?,?)",
+            # Customer 101's tier is NULL — the target-side nullable grain member.
+            [
+                (100, 1, "gold", 1000.0),
+                (101, 2, None, 250.0),
+                (102, 2, None, 75.0),
+            ],
+        )
+        con.executemany(
+            "INSERT INTO orders VALUES (?,?,?,?,?)",
+            [
+                (1, 100, "paid", "2024-01-15", PAID_JAN),
+                (2, 100, "paid", "2024-02-15", PAID_FEB),
+                # The NULL-status group — two months, so a 90-day window spans both.
+                (3, 101, None, "2024-01-20", NULL_STATUS_JAN),
+                (4, 101, None, "2024-02-20", NULL_STATUS_FEB),
+            ],
+        )
+        con.commit()
+    finally:
+        con.close()
 
 
 def dev1746_models() -> List[SlayerModel]:

tests/_engine_helpers.py

@@ -147,18 +147,20 @@ def _extract_src_body(sql: str) -> str:
     """Pull out the ``_src`` subquery body from a generated window-measure SQL.
 
     Resilient when the outer query also contains other LEFT JOIN (...) blocks
-    (e.g. cross-model measure subqueries): anchors on the unique ``\\n) AS _src``
-    suffix and reverse-searches for the matching ``LEFT JOIN (\\n`` before it.
+    (e.g. cross-model measure subqueries): anchors on the ``\\n) AS _src`` suffix
+    and reverse-searches for the matching ``LEFT JOIN (\\n`` before it. Multiple
+    windowed measures emit SIBLING ``) AS _src`` closes (never nested); anchor on
+    the LAST so the reverse search pairs it with the last measure's opening.
 
     The missing-anchor assertion is not reachable with today's generator output
-    (CodeRabbit): without it ``rfind`` returns ``-1`` and the helper silently
-    returns a slice from an arbitrary offset, so a future change to the join
-    keyword or its formatting would surface as a confusing assertion against the
-    wrong text rather than a clear failure here.
+    (CodeRabbit): without it the helper silently returns a slice from an
+    arbitrary offset, so a future change to the join keyword or its formatting
+    would surface as a confusing assertion against the wrong text rather than a
+    clear failure here.
     """
-    close = re.search(r"\n[ \t]*\) AS _src", sql)
-    assert close is not None, f"No `) AS _src` closing the _src subquery in:\n{sql}"
-    end = close.start()
+    closes = list(re.finditer(r"\n[ \t]*\) AS _src", sql))
+    assert closes, f"No `) AS _src` closing the _src subquery in:\n{sql}"
+    end = closes[-1].start()
     opens = list(re.finditer(r"LEFT JOIN \(\n", sql[:end]))
     assert opens, f"No `LEFT JOIN (` opening the _src subquery in:\n{sql}"
     return sql[opens[-1].end():end]

tests/_golden_harness.py

@@ -32,6 +32,7 @@
 import asyncio
 import json
 import os
+import re
 from pathlib import Path
 from typing import Awaitable, Callable, Dict, Iterable, Mapping, Optional
 
@@ -57,9 +58,23 @@ def render_value(value) -> str:
     return str(value)
 
 
+#: Absolute filesystem paths in an exception message — a tempdir, the repo
+#: root, a CI checkout (incl. single-segment mounts like ``/workspace``) — are
+#: volatile and would make a recorded RAISE differ run-to-run and
+#: machine-to-machine. Collapse each to ``<PATH>`` so the recorded message pins
+#: the FAILURE, not where the test ran. The leading boundary (not preceded by a
+#: word char, ``:`` or ``/``) leaves URLs (``https://…``) and compact SQL
+#: division (``a/b/c``) intact.
+_ABS_PATH_RE = re.compile(r"(?<![\w:/])/[\w.\-]+(?:/[\w.\-]+)*")
+
+
+def _redact_paths(message: str) -> str:
+    return _ABS_PATH_RE.sub("<PATH>", message)
+
+
 def record_raise(exc: BaseException) -> dict:
     """The structured form of a case that raised."""
-    return {"error": type(exc).__name__, "message": str(exc)}
+    return {"error": type(exc).__name__, "message": _redact_paths(str(exc))}
 
 
 def expected_keys(*, case_ids: Iterable[str], dialects: Iterable[str]) -> set:

tests/test_agg_render_spec.py

@@ -58,6 +58,7 @@
     ResolvedAggKwarg,
     SQLGenerator,
 )
+from slayer.sql.render.aggregates import resolve_agg_entry
 
 
 def _str_kwarg(value: str) -> ResolvedAggKwarg:
@@ -1094,8 +1095,6 @@ def _spec(self, aggregation: str, *, sql: str | None = "amount") -> AggRenderSpe
         )
 
     def test_simple_class_comes_from_the_registry_entry(self) -> None:
-        from slayer.sql.render.aggregates import resolve_agg_entry
-
         gen = SQLGenerator(dialect="postgres")
         expr, is_agg = gen._build_agg(self._spec("sum"))
         assert is_agg is True

tests/test_cross_model_rename_dev1448.py

@@ -1182,7 +1182,7 @@ async def test_filter_via_user_alias_resolves_to_cross_model_having(
             f"aggregate:\n{sql}"
         )
         # Never a WHERE against a column that doesn't exist on the base table.
-        assert "orders.cust_rev > 100" not in sql, (
+        assert "orders.cust_rev > 100" not in _norm(sql), (
             f"the filter must not be emitted against a non-existent base-table "
             f"column:\n{sql}"
         )

tests/test_dev1712_order_only_hidden_slots.py

@@ -366,7 +366,7 @@ async def test_two_order_terms_wrap_independently(self, engine) -> None:
         order_cols = [name for _, name in _outer_order_by_columns(sql)]
         assert order_cols == ["orders.created_at_max", "orders.rev"], sql
 
-    async def test_max_wrap_bypasses_the_aggregation_gate(self, engine) -> None:
+    async def test_wrap_bypasses_the_aggregation_gate(self, engine) -> None:
         """The hidden sort wrap is interned post-bind, so a column that does
         not whitelist ``max`` can still be sorted on — the caller asked to
         SORT, not to aggregate. ``id`` is a primary key (restricted to
@@ -385,9 +385,9 @@ async def test_max_wrap_bypasses_the_aggregation_gate(self, engine) -> None:
 
 
 # ===========================================================================
-# Group 3 — joined row column -> UnresolvableOrderColumnError.
+# Group 3 — joined row column -> Law-1 join pull / host-rooted CTE (resolves).
 # ===========================================================================
-class TestJoinedRowColumnRejected:
+class TestJoinedRowColumnResolved:
     async def test_joined_row_column_ungrouped_pulls_join_and_splits(
         self, engine,
     ) -> None:
@@ -430,8 +430,8 @@ async def test_joined_row_column_grouped_resolves_host_rooted(
         sql = await _sql(engine, query)
         assert _outer_select_columns(sql) == ["orders.status", "orders._count"], sql
         assert "WITH" in sql.upper(), sql
-        # DESC takes each group's MAXIMUM (D10).
-        assert re.search(r"(?i)\bMAX\s*\(", sql), sql
+        # DESC takes each group's MAXIMUM (D10) of the joined column.
+        assert re.search(r"MAX\(\s*customers\.region\s*\)", sql), sql
 
     async def test_ungrouped_order_by_derived_crossing_column_resolves(
         self, engine,
@@ -509,9 +509,12 @@ async def test_joined_order_ref_colliding_local_leaf_stays_joined(
             order=[OrderItem(column="owners.status", direction="desc")],  # joined
         )
         resp = await engine.execute(query, dry_run=True)
-        assert "owners" in (resp.sql or ""), (
-            f"joined order ref silently bound to the local column.\n"
-            f"SQL:\n{resp.sql}"
+        order_cols = _outer_order_by_columns(resp.sql or "")
+        assert any(
+            "owners" in table or "owners" in name for table, name in order_cols
+        ), (
+            f"joined order ref silently bound to the local column — the outer "
+            f"ORDER BY does not reference owners.\nSQL:\n{resp.sql}"
         )
 
 

tests/test_dev1733_order_only_transform_composite.py

@@ -1011,10 +1011,10 @@ async def test_non_sum_avg_windowed_order_target_still_raises(self, engine) -> N
 
 
 # ===========================================================================
-# Group 8 — shapes that must KEEP raising. Widening the hidden-order branch
-# must not swallow the Stage-8 rejections.
+# Group 8 — widening the hidden-order branch must not cross its boundaries:
+# some shapes must KEEP raising; the newly-resolved ones must not change grain.
 # ===========================================================================
-class TestStillRejected:
+class TestStillGuarded:
     async def test_joined_row_column_order_resolves_host_rooted(
         self, engine,
     ) -> None:
@@ -1033,6 +1033,18 @@ async def test_joined_row_column_order_resolves_host_rooted(
         assert [e.alias_or_name for e in parsed.expressions] == [
             "orders.status", "orders._count",
         ], sql
+        # Grain unchanged: the base CTE still groups on the query dim only —
+        # the joined sort key did NOT widen the base GROUP BY nor join the base.
+        base_cte = next(
+            (c.this for c in sqlglot.parse_one(sql, dialect="sqlite").find_all(exp.CTE)
+             if c.alias == "_base"), None,
+        )
+        assert base_cte is not None, f"expected a `_base` CTE.\nSQL:\n{sql}"
+        group = base_cte.args.get("group")
+        assert group is not None, f"`_base` has no GROUP BY.\nSQL:\n{sql}"
+        assert [g.name for g in group.expressions] == ["status"], (
+            f"base GROUP BY grain widened past the query dims.\nSQL:\n{sql}"
+        )
 
     async def test_ungrouped_row_column_still_splits(self, engine) -> None:
         """The DEV-1712 split-emission path must be untouched."""

tests/test_dev1744_naming_allocator.py

@@ -1011,6 +1011,8 @@ def _key(source, agg: str, *, args=(), kwargs=()) -> AggregateKey:
 # returns None there (it falls through to its own formula-text sanitiser,
 # which is NOT part of the aggregate-alias contract and stays in
 # stage_planner).
+# ``model_copy(update=...)`` deliberately bypasses validation: a TimeTruncKey
+# source is outside ``_AggregateSource`` and normal construction would reject it.
 _MISSING_LEAF_KEY = AggregateKey(
     source=ColumnKey(leaf="x"), agg="sum",
 ).model_copy(

tests/test_dev1744_value_expr.py

@@ -1343,8 +1343,8 @@ async def test_outer_wrapper_scalar_call_is_transpiled_on_postgres(
             ),
             dry_run=True,
         )
-        assert "IFNULL" not in resp.sql.upper(), resp.sql
-        assert "COALESCE" in resp.sql.upper(), resp.sql
+        assert "IFNULL" not in (resp.sql or "").upper(), resp.sql
+        assert "COALESCE" in (resp.sql or "").upper(), resp.sql
 
     async def test_shifted_cte_filter_call_site_executes(self, tmp_path_factory) -> None:
         """R1's SECOND call site (``_shifted_where_part``, the ``time_shift`` CTE's WHERE).
@@ -1965,7 +1965,7 @@ async def test_correct_arity_still_binds(self, e2e) -> None:
             ),
             dry_run=True,
         )
-        assert "ROUND" in resp.sql.upper()
+        assert "ROUND" in (resp.sql or "").upper()
 
 
 class TestNullInInList:

tests/test_dev1745_fragment_joins.py

@@ -20,6 +20,7 @@
 
 from __future__ import annotations
 
+import duckdb
 import pytest
 
 from slayer.core.enums import DataType
@@ -34,6 +35,7 @@
 from slayer.core.query import SlayerQuery
 from slayer.sql.generator import SQLGenerator
 
+from tests._dev1746_fixtures import cte_names_in_order, find_cte
 from tests._engine_helpers import _engine_generate
 
 
@@ -149,14 +151,24 @@ def _entered_fragments(*, kwargs, agg="sum") -> list:
         return seen
 
     def test_reserved_marker_kwarg_is_not_parsed_as_sql(self) -> None:
-        """``window='90d'`` is the standing example — a marker on a BUILT-IN
-        aggregation, which has no template to substitute it into."""
-        assert self._entered_fragments(kwargs=(("window", "90d"),)) == []
+        """``window='90d'`` is the standing example — a marker whose ``{window}``
+        never appears in ``wscaled_sum``'s template, so it is not a fragment.
+        Uses a TEMPLATED aggregation so the scan runs PAST the no-template
+        guard; the default ``w`` param still contributes ``regions.weight``, so
+        the check is that the MARKER's value is absent, not that nothing ran."""
+        entered = self._entered_fragments(
+            kwargs=(("window", "90d"),), agg="wscaled_sum",
+        )
+        assert "90d" not in entered, entered
 
     def test_marker_that_is_not_parseable_sql_is_still_skipped(self) -> None:
-        """The failure this guards: a marker whose text sqlglot rejects. It
-        must never reach the door, which raises."""
-        assert self._entered_fragments(kwargs=(("fmt", "%Y-%m"),)) == []
+        """The failure this guards: a marker whose text sqlglot rejects. Under a
+        TEMPLATED aggregation the substitution filter is what skips it, so it
+        never reaches the door (which would raise)."""
+        entered = self._entered_fragments(
+            kwargs=(("fmt", "%Y-%m"),), agg="wscaled_sum",
+        )
+        assert "%Y-%m" not in entered, entered
 
     def test_a_substituted_kwarg_is_still_scanned(self) -> None:
         """The counter-case, so the filter is not blanket suppression:
@@ -167,24 +179,22 @@ def test_a_substituted_kwarg_is_still_scanned(self) -> None:
         assert entered == ["regions.weight"], entered
 
 
-def _cm_body(sql: str) -> str:
-    """The body of the `_cm_` CTE.
+def _cm_body(sql: str, *, dialect: str = "postgres") -> str:
+    """The rendered body of the `_cm_` CTE.
 
     Assertions about which alias the fragment rendered belong to THIS scope:
     a whole-SQL check can be satisfied — or defeated — by a perfectly valid
     alias in the host base or the combined SELECT.
     """
-    start = sql.index("_cm_")
-    open_paren = sql.index("(", start)
-    depth = 0
-    for i in range(open_paren, len(sql)):
-        if sql[i] == "(":
-            depth += 1
-        elif sql[i] == ")":
-            depth -= 1
-            if depth == 0:
-                return sql[open_paren + 1:i]
-    raise AssertionError(f"unbalanced _cm_ CTE in:\n{sql}")
+    name = next(
+        (n for n in cte_names_in_order(sql, dialect=dialect)
+         if n.startswith("_cm_")),
+        None,
+    )
+    assert name is not None, f"no _cm_ CTE in:\n{sql}"
+    body = find_cte(sql, name, dialect=dialect)
+    assert body is not None, f"no _cm_ CTE in:\n{sql}"
+    return body.sql(dialect=dialect)
 
 
 @pytest.mark.asyncio
@@ -271,8 +281,6 @@ async def test_query_time_string_kwarg_override(self) -> None:
 @pytest.mark.asyncio
 async def test_cross_model_fragment_executes_on_duckdb() -> None:
     """A missing join is not a cosmetic difference — the SQL does not bind."""
-    import duckdb
-
     sql = await _sql(
         SlayerQuery(
             source_model="orders",