EnxDev commented on code in PR #45205:
URL: https://github.com/apache/superset/pull/45205#discussion_r4255951867
##########
tests/unit_tests/db_engine_specs/test_crdb.py:
##########
@@ -58,3 +58,41 @@ def test_dialect_loads_under_installed_sqlalchemy() -> None:
from sqlalchemy import create_engine
create_engine("cockroachdb://root@localhost:26257/defaultdb?sslmode=disable")
+
+
+def test_cockroachdb_properties() -> None:
+ from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+ from superset.db_engine_specs.postgres import PostgresEngineSpec
+
+ assert CockroachDbEngineSpec.engine == "cockroachdb"
+ assert CockroachDbEngineSpec.engine_name == "CockroachDB"
+ assert issubclass(CockroachDbEngineSpec, PostgresEngineSpec)
+ assert CockroachDbEngineSpec._extended_aggregations == {}
+
+
+def test_cockroachdb_metadata() -> None:
+ from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+
+ metadata = CockroachDbEngineSpec.metadata
+ assert "CockroachDB is a distributed SQL database" in
metadata["description"]
Review Comment:
Most of the `*_metadata` and `*_properties` tests in this PR restate
constants from the spec: logo filename, description text, `engine_name`. They
can only fail when someone edits that copy, and then the fix is to edit the
test to match.
I'd keep the ones that pin behaviour (`_extended_aggregations == {}` here
and on OceanBase, `_is_motherduck`, the time grain expressions) and drop the
string mirrors. Your call, but it would make the PR smaller and the signal
clearer.
##########
tests/unit_tests/db_engine_specs/test_crdb.py:
##########
@@ -58,3 +58,41 @@ def test_dialect_loads_under_installed_sqlalchemy() -> None:
from sqlalchemy import create_engine
create_engine("cockroachdb://root@localhost:26257/defaultdb?sslmode=disable")
+
+
+def test_cockroachdb_properties() -> None:
+ from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+ from superset.db_engine_specs.postgres import PostgresEngineSpec
+
+ assert CockroachDbEngineSpec.engine == "cockroachdb"
+ assert CockroachDbEngineSpec.engine_name == "CockroachDB"
+ assert issubclass(CockroachDbEngineSpec, PostgresEngineSpec)
+ assert CockroachDbEngineSpec._extended_aggregations == {}
+
+
+def test_cockroachdb_metadata() -> None:
+ from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+
+ metadata = CockroachDbEngineSpec.metadata
+ assert "CockroachDB is a distributed SQL database" in
metadata["description"]
+ assert metadata["logo"] == "cockroachdb.png"
+ assert "sqlalchemy-cockroachdb" in metadata["pypi_packages"]
+ assert metadata["default_port"] == 26257
+
+
[email protected](
+ "target_type,expected_result",
+ [
+ ("Date", "'2019-01-02'"),
+ ("DateTime", "'2019-01-02 03:04:05'"),
+ ("UnknownType", None),
+ ],
+)
+def test_cockroachdb_convert_dttm(
Review Comment:
This is `test_convert_dttm` above with one row changed. `Date` and
`UnknownType` are already covered there.
Could the `DateTime` row just join the existing parametrize? A string type
would be worth adding too, since `convert_dttm` also accepts `types.String` and
nothing exercises that branch.
--
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]
