mikebridge commented on code in PR #45395:
URL: https://github.com/apache/superset/pull/45395#discussion_r4342571407


##########
superset-core/src/superset_core/semantic_layers/testing.py:
##########
@@ -59,3 +79,277 @@ def semantic_layer_registration(monkeypatch: 
pytest.MonkeyPatch) -> None:
     integration lane. Use in isolated provider test processes, not a live host.
     """
     monkeypatch.setattr(decorators, "semantic_layer", _registration)
+
+
+# Keep test data and provider construction in the provider's own fixtures. This
+# suite consumes the real SDK types rather than a copy of a factory signature.
+class SemanticProviderConformance:
+    """Subclass in a ``Test...`` class and supply the documented provider 
fixtures.
+
+    Query fixtures seed discriminating data and provide independent expected
+    Arrow tables for identity, grain, NULL, grouped OR, and order-before-limit.
+    Error fixtures call real provider methods against controlled backend 
outcomes.
+    See the provider author guide for every fixture and the limits of this 
suite.
+    """
+
+    def test_catalog_identity(
+        self, semantic_view: SemanticView, semantic_reloaded_view: SemanticView
+    ) -> None:
+        """Stable selections survive reloads without ambiguous catalog 
members."""
+        assert semantic_view.uid()
+        assert semantic_view.uid() == semantic_reloaded_view.uid()
+        assert _catalog(semantic_view) == _catalog(semantic_reloaded_view)
+
+    @pytest.mark.parametrize(
+        "case", ["identity", "grain", "null", "not_null", "or", "order_limit"]
+    )
+    def test_query_outputs(
+        self,
+        case: str,
+        semantic_view: SemanticView,
+        semantic_query_cases: Mapping[str, tuple[SemanticQuery, pa.Table]],
+    ) -> None:
+        """Compare executed results, including column names and row order."""
+        query: SemanticQuery
+        expected: pa.Table
+        query, expected = semantic_query_cases[case]
+        assert expected.num_rows > 0, (
+            f"{case} must distinguish a correct result from no data"
+        )
+        assert query.order or expected.num_rows == 1, (
+            f"{case}: multi-row fixtures need explicit order"
+        )
+        assert set(query.dimensions) <= semantic_view.get_dimensions()
+        assert set(query.metrics) <= semantic_view.get_metrics()
+        if case == "grain":
+            assert any(dimension.grain is not None for dimension in 
query.dimensions)
+        elif case in {"null", "not_null"}:
+            operator: Operator = (
+                Operator.IS_NULL if case == "null" else Operator.IS_NOT_NULL
+            )
+            assert any(
+                isinstance(item, Filter) and item.operator == operator
+                for item in query.filters or set()
+            )
+        elif case == "or":
+            assert any(isinstance(item, OrFilter) for item in query.filters or 
set())
+        elif case == "order_limit":
+            assert query.order
+            assert query.limit is not None
+            assert query.limit > 0
+            full: SemanticResult = semantic_view.get_table(
+                replace(query, limit=None, offset=None)
+            )
+            assert full.results.num_rows > query.limit, "seed more rows than 
the limit"
+            _assert_result_equal(
+                full.results.slice(query.offset or 0, query.limit), expected
+            )
+        result: SemanticResult = semantic_view.get_table(query)
+        _assert_result_equal(result.results, expected)
+        assert set(result.results.column_names) == {
+            member.name for member in [*query.dimensions, *query.metrics]
+        }
+
+    @pytest.mark.parametrize("method", ["get_table", "get_values", 
"get_row_count"])
+    @pytest.mark.parametrize("kind", ["refusal", "fault", "incomplete", 
"unverified"])
+    def test_error_categories(
+        self,
+        method: str,
+        kind: str,
+        semantic_error_cases: Mapping[str, Callable[[], SemanticResult]],
+    ) -> None:
+        """Refusals and completeness are typed; operational faults stay 
faults."""
+        operation: Callable[[], SemanticResult] = semantic_error_cases[
+            f"{method}:{kind}"
+        ]
+        error: pytest.ExceptionInfo[Exception]
+        # Operational fault classes are provider-owned; classify the caught 
error below.
+        with pytest.raises(Exception) as error:  # noqa: PT011
+            operation()
+        if kind == "refusal":
+            assert isinstance(error.value, SemanticQueryRejectedError)
+            assert isinstance(error.value.code, SemanticQueryErrorCode)
+        elif kind in {"incomplete", "unverified"}:
+            assert isinstance(error.value, SemanticResultCompletenessError)
+            assert error.value.reason == kind
+        else:
+            assert not isinstance(
+                error.value,
+                (TypeError, AttributeError, NameError, AssertionError, 
KeyError),
+            ), f"Fault fixture raised a programming error: {error.value!r}"
+            assert not isinstance(
+                error.value,
+                (SemanticQueryRejectedError, SemanticResultCompletenessError),
+            )
+
+    @pytest.mark.parametrize("feature", list(SemanticViewFeature))
+    def test_advertised_capabilities(
+        self,
+        feature: SemanticViewFeature,
+        semantic_view: SemanticView,
+        semantic_capability_cases: Mapping[str, tuple[SemanticQuery, 
pa.Table]],
+    ) -> None:
+        """Every advertised feature has its own result-level witness."""
+        if feature not in semantic_view.features:
+            pytest.skip(f"Provider does not advertise {feature.value}")
+        query: SemanticQuery
+        expected: pa.Table
+        query, expected = semantic_capability_cases[feature.value]
+        assert expected.num_rows > 0
+        assert query.order or expected.num_rows == 1, (
+            f"{feature.value}: multi-row fixtures need explicit order"
+        )
+        if feature == SemanticViewFeature.ROW_OFFSET:
+            assert query.offset is not None
+            assert query.offset > 0
+            assert query.limit is not None
+            assert query.limit > 0
+        elif feature in {
+            SemanticViewFeature.GROUP_LIMIT,
+            SemanticViewFeature.GROUP_OTHERS,
+        }:
+            assert query.group_limit is not None
+            if feature == SemanticViewFeature.GROUP_OTHERS:
+                assert query.group_limit.group_others
+        elif feature == SemanticViewFeature.ADHOC_COLUMN_EXPRESSIONS:
+            assert any(
+                isinstance(member, AdhocExpression) for member in 
query.dimensions
+            ), "ADHOC_COLUMN_EXPRESSIONS requires an AdhocExpression dimension"
+        elif feature == SemanticViewFeature.ADHOC_EXPRESSIONS_IN_ORDERBY:
+            assert any(
+                isinstance(member, AdhocExpression) for member, _ in 
query.order or []
+            ), "ADHOC_EXPRESSIONS_IN_ORDERBY requires an AdhocExpression order 
key"
+        _assert_result_equal(semantic_view.get_table(query).results, expected)
+
+    def test_metadata_generation(
+        self,
+        semantic_make_view: Callable[[str], SemanticView],
+        semantic_change_catalog: Callable[[], None],
+    ) -> None:
+        """Changed generations cannot reuse a stale catalog.
+
+        The change fixture must add a member on each call. Factories and 
transport
+        stay provider-owned; uncached legacy providers may report a None token.
+        """
+        first: SemanticView = semantic_make_view("generation-a")
+        before: tuple[set[str], set[tuple[str, str | None]]] = _catalog(first)
+        assert first.metadata_cache_token in {None, "generation-a"}
+        semantic_change_catalog()
+        second: SemanticView = semantic_make_view("generation-b")
+        after: tuple[set[str], set[tuple[str, str | None]]] = _catalog(second)
+        assert after != before

Review Comment:
   Fixed in 06568a7c8a8a57e50f5efac3435eb05cc42a2c9d. Both the metric names and 
dimension name/grain pairs must survive the generation change, and at least one 
member must be added. The example retains its earlier revision members instead 
of replacing them. Regressions reject an empty generation-B catalog, a shrunken 
one and a replacement-only catalog. The guide also makes clear that legacy None 
echoes do not verify token relabeling.



##########
superset-core/src/superset_core/semantic_layers/testing.py:
##########
@@ -59,3 +79,277 @@ def semantic_layer_registration(monkeypatch: 
pytest.MonkeyPatch) -> None:
     integration lane. Use in isolated provider test processes, not a live host.
     """
     monkeypatch.setattr(decorators, "semantic_layer", _registration)
+
+
+# Keep test data and provider construction in the provider's own fixtures. This
+# suite consumes the real SDK types rather than a copy of a factory signature.
+class SemanticProviderConformance:
+    """Subclass in a ``Test...`` class and supply the documented provider 
fixtures.
+
+    Query fixtures seed discriminating data and provide independent expected
+    Arrow tables for identity, grain, NULL, grouped OR, and order-before-limit.
+    Error fixtures call real provider methods against controlled backend 
outcomes.
+    See the provider author guide for every fixture and the limits of this 
suite.
+    """
+
+    def test_catalog_identity(
+        self, semantic_view: SemanticView, semantic_reloaded_view: SemanticView
+    ) -> None:
+        """Stable selections survive reloads without ambiguous catalog 
members."""
+        assert semantic_view.uid()
+        assert semantic_view.uid() == semantic_reloaded_view.uid()
+        assert _catalog(semantic_view) == _catalog(semantic_reloaded_view)
+
+    @pytest.mark.parametrize(
+        "case", ["identity", "grain", "null", "not_null", "or", "order_limit"]
+    )
+    def test_query_outputs(
+        self,
+        case: str,
+        semantic_view: SemanticView,
+        semantic_query_cases: Mapping[str, tuple[SemanticQuery, pa.Table]],
+    ) -> None:
+        """Compare executed results, including column names and row order."""
+        query: SemanticQuery
+        expected: pa.Table
+        query, expected = semantic_query_cases[case]
+        assert expected.num_rows > 0, (
+            f"{case} must distinguish a correct result from no data"
+        )
+        assert query.order or expected.num_rows == 1, (
+            f"{case}: multi-row fixtures need explicit order"
+        )
+        assert set(query.dimensions) <= semantic_view.get_dimensions()
+        assert set(query.metrics) <= semantic_view.get_metrics()
+        if case == "grain":
+            assert any(dimension.grain is not None for dimension in 
query.dimensions)
+        elif case in {"null", "not_null"}:
+            operator: Operator = (
+                Operator.IS_NULL if case == "null" else Operator.IS_NOT_NULL
+            )
+            assert any(
+                isinstance(item, Filter) and item.operator == operator
+                for item in query.filters or set()
+            )
+        elif case == "or":
+            assert any(isinstance(item, OrFilter) for item in query.filters or 
set())
+        elif case == "order_limit":
+            assert query.order
+            assert query.limit is not None
+            assert query.limit > 0
+            full: SemanticResult = semantic_view.get_table(
+                replace(query, limit=None, offset=None)
+            )
+            assert full.results.num_rows > query.limit, "seed more rows than 
the limit"
+            _assert_result_equal(
+                full.results.slice(query.offset or 0, query.limit), expected
+            )
+        result: SemanticResult = semantic_view.get_table(query)
+        _assert_result_equal(result.results, expected)
+        assert set(result.results.column_names) == {
+            member.name for member in [*query.dimensions, *query.metrics]
+        }
+
+    @pytest.mark.parametrize("method", ["get_table", "get_values", 
"get_row_count"])
+    @pytest.mark.parametrize("kind", ["refusal", "fault", "incomplete", 
"unverified"])
+    def test_error_categories(
+        self,
+        method: str,
+        kind: str,
+        semantic_error_cases: Mapping[str, Callable[[], SemanticResult]],
+    ) -> None:
+        """Refusals and completeness are typed; operational faults stay 
faults."""
+        operation: Callable[[], SemanticResult] = semantic_error_cases[
+            f"{method}:{kind}"
+        ]
+        error: pytest.ExceptionInfo[Exception]
+        # Operational fault classes are provider-owned; classify the caught 
error below.
+        with pytest.raises(Exception) as error:  # noqa: PT011
+            operation()
+        if kind == "refusal":
+            assert isinstance(error.value, SemanticQueryRejectedError)
+            assert isinstance(error.value.code, SemanticQueryErrorCode)
+        elif kind in {"incomplete", "unverified"}:
+            assert isinstance(error.value, SemanticResultCompletenessError)
+            assert error.value.reason == kind
+        else:
+            assert not isinstance(
+                error.value,
+                (TypeError, AttributeError, NameError, AssertionError, 
KeyError),
+            ), f"Fault fixture raised a programming error: {error.value!r}"
+            assert not isinstance(
+                error.value,
+                (SemanticQueryRejectedError, SemanticResultCompletenessError),
+            )
+
+    @pytest.mark.parametrize("feature", list(SemanticViewFeature))
+    def test_advertised_capabilities(
+        self,
+        feature: SemanticViewFeature,
+        semantic_view: SemanticView,
+        semantic_capability_cases: Mapping[str, tuple[SemanticQuery, 
pa.Table]],
+    ) -> None:
+        """Every advertised feature has its own result-level witness."""
+        if feature not in semantic_view.features:
+            pytest.skip(f"Provider does not advertise {feature.value}")
+        query: SemanticQuery
+        expected: pa.Table
+        query, expected = semantic_capability_cases[feature.value]
+        assert expected.num_rows > 0
+        assert query.order or expected.num_rows == 1, (
+            f"{feature.value}: multi-row fixtures need explicit order"
+        )
+        if feature == SemanticViewFeature.ROW_OFFSET:
+            assert query.offset is not None
+            assert query.offset > 0
+            assert query.limit is not None
+            assert query.limit > 0
+        elif feature in {
+            SemanticViewFeature.GROUP_LIMIT,
+            SemanticViewFeature.GROUP_OTHERS,
+        }:
+            assert query.group_limit is not None
+            if feature == SemanticViewFeature.GROUP_OTHERS:
+                assert query.group_limit.group_others
+        elif feature == SemanticViewFeature.ADHOC_COLUMN_EXPRESSIONS:
+            assert any(
+                isinstance(member, AdhocExpression) for member in 
query.dimensions

Review Comment:
   Agreed; fixed in 06568a7c8a8a57e50f5efac3435eb05cc42a2c9d. That capability 
case explicitly skips before fixture lookup because there is no SDK query shape 
for it. This leaves SemanticQuery.dimensions and the host contract unchanged, 
while keeping ADHOC_EXPRESSIONS_IN_ORDERBY tested through its valid expression 
type. The skip is reported as unverified coverage, not a passing capability.



##########
superset-core/tests/semantic_layers/test_conformance.py:
##########
@@ -0,0 +1,414 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from collections.abc import Callable
+from dataclasses import replace
+from datetime import date
+from functools import partial
+from pathlib import Path
+from typing import TYPE_CHECKING
+
+import pyarrow as pa
+import pytest
+from superset_core.semantic_layers.errors import 
SemanticResultCompletenessReason
+from superset_core.semantic_layers.testing import SemanticProviderConformance
+from superset_core.semantic_layers.types import (
+    Dimension,
+    Filter,
+    Grains,
+    Metric,
+    Operator,
+    OrderDirection,
+    OrFilter,
+    PredicateType,
+    SemanticQuery,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView, 
SemanticViewFeature
+
+if TYPE_CHECKING:
+    from conformance_provider import Backend, QueryView
+
+pytest_plugins: list[str] = ["superset_core.semantic_layers.testing"]
+
+
[email protected]
+def backend(semantic_layer_registration: None) -> "Backend":
+    """Create independent backend state for each conformance test."""
+    from conformance_provider import Backend
+
+    return Backend()
+
+
[email protected]
+def semantic_view(backend: "Backend") -> "QueryView":
+    """Bind the real SDK example view to its controlled backend."""
+    from conformance_provider import QueryView
+
+    return QueryView(backend)
+
+
[email protected]
+def semantic_reloaded_view(backend: "Backend") -> "QueryView":
+    """Reconstruct metadata through the provider's public catalog methods."""
+    from conformance_provider import QueryView
+
+    return QueryView(backend)
+
+
[email protected]
+def semantic_query_cases(
+    semantic_view: "QueryView",
+) -> dict[str, tuple[SemanticQuery, pa.Table]]:
+    """Expected answers are independent of the provider's query 
implementation."""
+    category: Dimension = next(
+        d for d in semantic_view.get_dimensions() if d.name == "category"
+    )
+    month: Dimension = next(
+        d for d in semantic_view.get_dimensions() if d.grain == Grains.MONTH
+    )
+    total: Metric = next(iter(semantic_view.get_metrics()))
+    null: Filter = Filter(PredicateType.WHERE, category, Operator.IS_NULL, 
None)
+    equals: Filter = Filter(PredicateType.WHERE, category, Operator.EQUALS, 
"A")
+    return {
+        "identity": (
+            SemanticQuery([total], [category], order=[(total, 
OrderDirection.ASC)]),
+            pa.table(
+                {
+                    "category": pa.array([None, "A", "B"], type=pa.string()),
+                    "total": [3, 7, 8],
+                }
+            ),
+        ),
+        "grain": (
+            SemanticQuery([total], [month], order=[(month, 
OrderDirection.ASC)]),
+            pa.table(
+                {
+                    "occurred": pa.array(
+                        [date(2026, 1, 1), date(2026, 2, 1)], type=pa.date32()
+                    ),
+                    "total": [5, 13],
+                }
+            ),
+        ),
+        "null": (
+            SemanticQuery([total], [category], filters={null}),
+            pa.table({"category": pa.array([None], type=pa.string()), "total": 
[3]}),
+        ),
+        "not_null": (
+            SemanticQuery(
+                [total],
+                [category],
+                filters={
+                    Filter(PredicateType.WHERE, category, 
Operator.IS_NOT_NULL, None)
+                },
+                order=[(total, OrderDirection.ASC)],
+            ),
+            pa.table({"category": ["A", "B"], "total": [7, 8]}),
+        ),
+        "or": (
+            SemanticQuery(
+                [total],
+                [category],
+                filters={OrFilter(frozenset({null, equals}))},

Review Comment:
   Fixed in 06568a7c8a8a57e50f5efac3435eb05cc42a2c9d. The witness combines 
normalized IN/IS_NULL with an outer date comparison, with both A and NULL 
records on either side of the date boundary. Removing parentheses changes the 
result whether the OR group is rendered before or after the outer AND. The 
regression executes both orders; dropping the outer comparison or one IN value 
also fails.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to