FrancescoCastaldi commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r3940220981


##########
tests/unit_tests/db_engine_specs/test_ocient.py:
##########
@@ -14,404 +14,55 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-
-# pylint: disable=import-outside-toplevel
-
-from typing import Any, Callable
-
 import pytest
 
-from superset.db_engine_specs.ocient import (
-    _point_list_to_wkt,
-    _sanitized_ocient_type_codes,
-)
-from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-
-
-def ocient_is_installed() -> bool:
-    return len(_sanitized_ocient_type_codes) > 0
-
+from superset.constants import TimeGrain
+from superset.db_engine_specs.ocient import OcientEngineSpec
 
-# (msg,expected)
-MARSHALED_OCIENT_ERRORS: list[tuple[str, SupersetError]] = [
-    (
-        "The referenced user does not exist (User 'mj' not found)",
-        SupersetError(
-            message='The username "mj" does not exist.',
-            error_type=SupersetErrorType.CONNECTION_INVALID_USERNAME_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1012,
-                        "message": "Issue 1012 - The username provided when 
connecting to a database is not valid.",  # noqa: E501
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "The userid/password combination was not valid (Incorrect password for 
user)",
-        SupersetError(
-            message="The user/password combination is not valid (Incorrect 
password for user).",  # noqa: E501
-            error_type=SupersetErrorType.CONNECTION_INVALID_PASSWORD_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1013,
-                        "message": "Issue 1013 - The password provided when 
connecting to a database is not valid.",  # noqa: E501
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "No database named 'bulls' exists",
-        SupersetError(
-            message='Could not connect to database: "bulls"',
-            error_type=SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1015,
-                        "message": "Issue 1015 - Either the database is 
spelled incorrectly or does not exist.",  # noqa: E501
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "Unable to connect to unitedcenter.com:4050",
-        SupersetError(
-            message='Could not resolve hostname: "unitedcenter.com".',
-            error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1007,
-                        "message": "Issue 1007 - The hostname provided can't 
be resolved.",  # noqa: E501
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "Port out of range 0-65535",
-        SupersetError(
-            message="Port out of range 0-65535",
-            error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1034,
-                        "message": "Issue 1034 - The port number is invalid.",
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "An invalid connection string attribute was specified (failed to 
decrypt cipher text)",  # noqa: E501
-        SupersetError(
-            message="Invalid Connection String: Expecting String of the form 
'ocient://user:pass@host:port/database'.",
-            error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1002,
-                        "message": "Issue 1002 - The database returned an 
unexpected error.",  # noqa: E501
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "There is a syntax error in your statement (extraneous input 'foo bar 
baz' expecting {<EOF>, 'trace', 'using'})",  # noqa: E501
-        SupersetError(
-            message="Syntax Error: extraneous input \"foo bar baz\" expecting 
\"{<EOF>, 'trace', 'using'}",  # noqa: E501
-            error_type=SupersetErrorType.SYNTAX_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1030,
-                        "message": "Issue 1030 - The query has a syntax 
error.",
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "There is a syntax error in your statement (mismatched input 'to' 
expecting {<EOF>, 'trace', 'using'})",  # noqa: E501
-        SupersetError(
-            message="Syntax Error: mismatched input \"to\" expecting \"{<EOF>, 
'trace', 'using'}",  # noqa: E501
-            error_type=SupersetErrorType.SYNTAX_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1030,
-                        "message": "Issue 1030 - The query has a syntax 
error.",
-                    }
-                ],
-            },
-        ),
-    ),
-    (
-        "The referenced table or view 'goats' does not exist",
-        SupersetError(
-            message='Table or View "goats" does not exist.',
-            error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1003,
-                        "message": "Issue 1003 - There is a syntax error in 
the SQL query. Perhaps there was a misspelling or a typo.",  # noqa: E501
-                    },
-                    {
-                        "code": 1005,
-                        "message": "Issue 1005 - The table was deleted or 
renamed in the database.",  # noqa: E501
-                    },
-                ],
-            },
-        ),
-    ),
-    (
-        "The reference to column 'goats' is not valid",
-        SupersetError(
-            message='Invalid reference to column: "goats"',
-            error_type=SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR,
-            level=ErrorLevel.ERROR,
-            extra={
-                "engine_name": "Ocient",
-                "issue_codes": [
-                    {
-                        "code": 1003,
-                        "message": "Issue 1003 - There is a syntax error in 
the SQL query. Perhaps there was a misspelling or a typo.",  # noqa: E501
-                    },
-                    {
-                        "code": 1004,
-                        "message": "Issue 1004 - The column was deleted or 
renamed in the database.",  # noqa: E501
-                    },
-                ],
-            },
-        ),
-    ),
-]
 
+def test_ocient_properties() -> None:
+    assert OcientEngineSpec.engine == "ocient"
+    assert OcientEngineSpec.engine_name == "Ocient"
+    assert OcientEngineSpec.force_column_alias_quotes is True
+    assert OcientEngineSpec.max_column_name_length == 30
+    assert OcientEngineSpec.cte_alias == "cte__"
 
[email protected]("msg,expected", MARSHALED_OCIENT_ERRORS)
-def test_connection_errors(msg: str, expected: SupersetError) -> None:
-    from superset.db_engine_specs.ocient import OcientEngineSpec
 
-    result = OcientEngineSpec.extract_errors(Exception(msg))
-    assert result == [expected]
+def test_ocient_metadata() -> None:

Review Comment:
   @sadpandajoe Good catch! Restored all original Ocient error mapping, GIS 
type sanitization, and point-list-to-WKT tests alongside metadata suite in 
commit 601005b.



##########
tests/unit_tests/db_engine_specs/test_parseable.py:
##########
@@ -15,46 +15,36 @@
 # specific language governing permissions and limitations
 # under the License.
 from datetime import datetime
-from typing import Optional
 
 import pytest
 
+from superset.constants import TimeGrain
+from superset.db_engine_specs.parseable import ParseableEngineSpec
 from tests.unit_tests.db_engine_specs.utils import assert_convert_dttm
 from tests.unit_tests.fixtures.common import dttm  # noqa: F401
 
 
-def test_epoch_to_dttm() -> None:
-    """
-    DB Eng Specs (parseable): Test epoch to dttm
-    """
-    from superset.db_engine_specs.parseable import ParseableEngineSpec
-
-    assert ParseableEngineSpec.epoch_to_dttm() == "to_timestamp({col})"
-
+def test_parseable_properties() -> None:
+    assert ParseableEngineSpec.engine == "parseable"
+    assert ParseableEngineSpec.engine_name == "Parseable"
 
-def test_epoch_ms_to_dttm() -> None:
-    """
-    DB Eng Specs (parseable): Test epoch ms to dttm
-    """
-    from superset.db_engine_specs.parseable import ParseableEngineSpec
 
-    assert ParseableEngineSpec.epoch_ms_to_dttm() == "to_timestamp({col} / 
1000)"
+def test_parseable_metadata() -> None:

Review Comment:
   @sadpandajoe Thanks for pointing that out! Restored the original 
`test_alter_new_orm_column` test for `p_timestamp` datetime handling and 
`epoch_ms` format alongside the new metadata suite in commit 601005b.



-- 
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