sadpandajoe commented on code in PR #45395: URL: https://github.com/apache/superset/pull/45395#discussion_r4332112675
########## 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: The only `OrFilter` case has no outer AND, so the example's `_predicate` can drop the parentheses around the OR group and this case still returns `[(None, 3), ('A', 7)]`. With an added outer `IS_NOT_NULL(category)`, the correct result is only `('A', 7)`, while `category IS NOT NULL AND category = ? OR category IS NULL` also returns `(None, 3)`. Since the docs describe grouped-OR as a checked behavior, could this case be combined with an outer AND filter so a precedence bug is detected? ########## 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: `after != before` also passes when generation B comes back with an empty or shrunken catalog (for example both sets empty after a failed reload), and nothing asserts that the newly added member appears or that existing ones survive. A provider that drops its catalog on a generation change would therefore pass this lane. The docstring says the change fixture must add a member, and the example's `revision_0` -> `revision_1` swap replaces rather than adds. Could this assert that `before` is a strict subset of `after` (with the added member present), and add a regression that an empty generation-B catalog fails? ########## 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: This requires an `AdhocExpression` inside `query.dimensions`, but `SemanticQuery.dimensions` is `list[Dimension]` and `AdhocExpression` is a separate type that the SDK only accepts in ordering. A provider that advertises `ADHOC_COLUMN_EXPRESSIONS` can only satisfy this assertion by building a query that violates the SDK's own type contract, and a fixture written to the contract fails here before the provider is ever executed. Should this lane wait until the SDK defines what an ad-hoc column looks like, instead of asserting a shape the host contract does not define? -- 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]
