sadpandajoe commented on code in PR #43502: URL: https://github.com/apache/superset/pull/43502#discussion_r3857488829
########## tests/testcontainers/db_engine_specs/test_cockroachdb.py: ########## @@ -0,0 +1,89 @@ +# 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. +""" +Tests db_engine_specs.cockroachdb against a real CockroachDB instance, +spun up on demand via testcontainers. Run nightly (see +.github/workflows/nightly-testcontainers.yml), not on every merge -- these +exercise real SQL execution and dialect introspection, which mocked unit +tests structurally cannot. +""" + +from collections.abc import Iterator + +import pytest +from sqlalchemy import ( + Column, + create_engine, + inspect, + Integer, + MetaData, + Table as SATable, +) +from sqlalchemy.engine import Engine + +from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec +from superset.sql.parse import Table + +pytest.importorskip("testcontainers.community.cockroachdb") + +from testcontainers.community.cockroachdb import CockroachDBContainer # noqa: E402 + +from ._pagination import assert_paginated_query_returns_correct_rows_in_order + + [email protected](scope="module") +def engine() -> Iterator[Engine]: + # sqlalchemy-cockroachdb registers its dialect under the plain + # "cockroachdb" name; the container's own default ("cockroachdb+psycopg2") + # matches the abandoned `cockroachdb` package instead (see #43501). + with CockroachDBContainer(dialect="cockroachdb") as container: + yield create_engine(container.get_connection_url()) + + +def test_paginated_query_returns_correct_rows_in_order(engine: Engine) -> None: + """ + A plain SQLAlchemy Core LIMIT/OFFSET query, compiled and executed against + a real instance. Mocked tests cannot catch a dialect compiling this + incorrectly (see apache/superset#42899, where Trino emitted OFFSET + before LIMIT) -- only real execution can. + """ + assert_paginated_query_returns_correct_rows_in_order(engine) + + +def test_get_columns_maps_native_types(engine: Engine) -> None: + """ + CockroachDbEngineSpec.get_columns wraps a real SQLAlchemy Inspector; + this exercises that against actual server-reported column metadata + rather than a mocked Inspector. + """ + metadata = MetaData() + SATable( + "pilot_types", + metadata, + Column("id", Integer, primary_key=True), + Column("amount", Integer), + ) + metadata.create_all(engine) + + inspector = inspect(engine) + columns = CockroachDbEngineSpec.get_columns(inspector, Table("pilot_types")) + + by_name = {col["column_name"]: col for col in columns} + assert set(by_name) == {"id", "amount"} + for col in by_name.values(): + spec = CockroachDbEngineSpec.get_column_spec(str(col["type"])) + assert spec is not None Review Comment: This accepts any recognized type, so an inspector regression that maps `INTEGER` to a string or temporal type still passes even though the test claims to verify native-type mapping. Could this assert the expected generic and SQLAlchemy types for `id`/`amount`, as the Trino test does? -- 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]
