Refactoring/654 mark tests for new features (#655)

* Change marking for tests added for new features which are not yet implemented in sqlalchemy-exasol

* Upon checking what needed to be done to resolve these, it was found that test_get_multi_pk_constraint fully succeeds

* Resolve test_get_view_definition_does_not_exist by raising error when view does not exist

* Refactor names in base.py to be clearer

* Fix test_get_multi_columns as resolvable due to differing nullable expectation only

* Fix test_get_multi_foreign_keys as resolvable with new sorting, as custom constraints cannot be used in Exasol

* Fix test as now throws exception if view does not exist

* Switch skipif to xfail

* Add missing strict=True for xfail

* Apply review comments to improve comments for modifications

Author: ArBridgeman

doc/changes/unreleased.md

@@ -20,3 +20,8 @@ strings should be altered to start with `exa+websocket://`.
   - `ComponentReflectionTest.test_not_existing_table` is used to indicate that specific `EXADialect` methods (i.e. `get_columns`) check to see if the requested table/view exists and if not, they will now toss a `NoSuchTableError` exception
 - #403: Dropped support for Turbodbc
 - #404: Dropped support for pyodbc
+- #654: Reinstated `sqlalchemy` tests after minor modifications to work for Exasol:
+  - `ComponentReflectionTest.test_get_multi_columns`
+  - `ComponentReflectionTest.test_get_multi_foreign_keys`
+  - `ComponentReflectionTest.test_get_multi_pk_constraint`
+  - `ComponentReflectionTest.test_get_view_definition_does_not_exist`

sqlalchemy_exasol/base.py

@@ -934,7 +934,10 @@ def get_table_names(self, connection, schema, **kw):
         tables = [self.normalize_name(row[0]) for row in result]
         return tables
 
-    def has_table(self, connection, table_name, schema=None, **kw):
+    @reflection.cache
+    def has_table(self, connection, table_name, schema=None, **kw) -> bool:
+        self._ensure_has_table_connection(connection)
+
         schema = self._get_schema_for_input(connection, schema)
         sql_statement = (
             "SELECT OBJECT_NAME FROM SYS.EXA_ALL_OBJECTS "
@@ -956,34 +959,38 @@ def has_table(self, connection, table_name, schema=None, **kw):
 
     @reflection.cache
     def get_view_names(self, connection, schema=None, **kw):
-        schema = self._get_schema_for_input(connection, schema)
+        schema_name = self._get_schema_for_input(connection, schema)
         sql_statement = "SELECT view_name FROM  SYS.EXA_ALL_VIEWS WHERE view_schema = "
-        if schema is None:
+        if schema_name is None:
             sql_statement += "CURRENT_SCHEMA ORDER BY view_name"
             result = connection.execute(sql.text(sql_statement))
         else:
             sql_statement += ":schema ORDER BY view_name"
             result = connection.execute(
-                sql.text(sql_statement), {"schema": self.denormalize_name(schema)}
+                sql.text(sql_statement), {"schema": self.denormalize_name(schema_name)}
             )
         return [self.normalize_name(row[0]) for row in result]
 
     @reflection.cache
     def get_view_definition(self, connection, view_name, schema=None, **kw):
-        schema = self._get_schema_for_input(connection, schema)
-        sql_stmnt = "SELECT view_text FROM sys.exa_all_views WHERE view_name = :view_name AND view_schema = "
-        if schema is None:
-            sql_stmnt += "CURRENT_SCHEMA"
+        schema_name = self._get_schema_for_input(connection, schema)
+        sql_statement = "SELECT view_text FROM sys.exa_all_views WHERE view_name = :view_name AND view_schema = "
+        if schema_name is None:
+            sql_statement += "CURRENT_SCHEMA"
         else:
-            sql_stmnt += ":schema"
+            sql_statement += ":schema"
         result = connection.execute(
-            sql.text(sql_stmnt),
+            sql.text(sql_statement),
             {
                 "view_name": self.denormalize_name(view_name),
-                "schema": self.denormalize_name(schema),
+                "schema": self.denormalize_name(schema_name),
             },
         ).scalar()
-        return result if result else None
+        if result:
+            return result
+        raise sqlalchemy.exc.NoSuchTableError(
+            f"{schema_name}.{view_name}" if schema_name else view_name
+        )
 
     @staticmethod
     def quote_string_value(string_value):

test/integration/exasol/test_certificate.py

@@ -12,10 +12,6 @@
 from sqlalchemy.testing import config
 from sqlalchemy.testing.fixtures import TestBase
 
-FINGERPRINT_SECURITY_RATIONALE = (
-    "Only websocket supports giving a fingerprint in the connection"
-)
-
 
 def get_fingerprint(dsn):
     import websocket
@@ -60,9 +56,10 @@ def remove_ssl_settings(url):
             pass
         return url.set(query=query)
 
-    @pytest.mark.skipif(
+    @pytest.mark.xfail(
         testing.db.dialect.server_version_info < (7, 1, 0),
         reason="DB version(s) before 7.1.0 don't enforce ssl/tls",
+        strict=True,
     )
     def test_db_connection_fails_with_default_settings_for_self_signed_certificates(
         self,
@@ -79,10 +76,6 @@ def test_db_connection_fails_with_default_settings_for_self_signed_certificates(
         expected_substrings = ["self-signed certificate", "self signed certificate"]
         assert any([e in actual_message for e in expected_substrings])
 
-    @pytest.mark.skipif(
-        "websocket" not in testing.db.dialect.driver,
-        reason="Only websocket supports passing on connect_args like this.",
-    )
     def test_db_skip_certification_validation_passes(self):
         url = self.remove_ssl_settings(config.db.url)
 
@@ -99,10 +92,6 @@ def test_db_with_ssl_verify_none_passes(self):
         result = self.perform_test_query(engine)
         assert result == [(42,)]
 
-    @pytest.mark.skipif(
-        "websocket" not in testing.db.dialect.driver,
-        reason=FINGERPRINT_SECURITY_RATIONALE,
-    )
     def test_db_with_fingerprint_passes(self):
         url = self.remove_ssl_settings(config.db.url)
         connect_args = url.translate_connect_args(database="schema")
@@ -116,10 +105,6 @@ def test_db_with_fingerprint_passes(self):
         result = self.perform_test_query(engine)
         assert result == [(42,)]
 
-    @pytest.mark.skipif(
-        "websocket" not in testing.db.dialect.driver,
-        reason=FINGERPRINT_SECURITY_RATIONALE,
-    )
     def test_db_with_wrong_fingerprint_fails(self):
         url = self.remove_ssl_settings(config.db.url)
         connect_args = url.translate_connect_args(database="schema")

test/integration/exasol/test_deadlock.py

@@ -2,7 +2,6 @@
 from threading import Thread
 
 import pytest
-import sqlalchemy.testing as testing
 from sqlalchemy import (
     create_engine,
     inspect,

test/integration/exasol/test_get_metadata_functions.py

@@ -200,12 +200,12 @@ def test_get_view_definition(self, engine_name):
     def test_get_view_definition_view_name_none(self, engine_name):
         with self.engine_map[engine_name].begin() as c:
             dialect = inspect(c).dialect
-            view_definition = dialect.get_view_definition(
-                connection=c,
-                schema=self.schema,
-                view_name=None,
-            )
-            assert view_definition is None
+            with pytest.raises(NoSuchTableError):
+                dialect.get_view_definition(
+                    connection=c,
+                    schema=self.schema,
+                    view_name=None,
+                )
 
     @pytest.mark.parametrize(
         "engine_name",

test/integration/sqlalchemy/test_suite.py

@@ -5,6 +5,7 @@
 import pytest
 import sqlalchemy as sa
 from pyexasol import ExaQueryError
+from sqlalchemy import Inspector
 from sqlalchemy.schema import (
     DDL,
     Index,
@@ -23,6 +24,7 @@
 from sqlalchemy.testing.suite import ReturningGuardsTest as _ReturningGuardsTest
 from sqlalchemy.testing.suite import RowCountTest as _RowCountTest
 from sqlalchemy.testing.suite import RowFetchTest as _RowFetchTest
+from sqlalchemy.testing.suite.test_reflection import _multi_combination
 
 """
 Here, all tests are imported from the testing suite of sqlalchemy to ensure that the
@@ -334,25 +336,100 @@ class sqlalchemy.testing.suite.ComponentReflectionTest
         if not schema and testing.requires.temp_table_reflection.enabled:
             cls.define_temp_tables(metadata)
 
-    @pytest.mark.xfail(reason=XfailRationale.MANUAL_INDEX.value)
+    @staticmethod
+    def _convert_view_nullable(expected_multi_output):
+        """
+        Convert expected nullable to None
+
+        For columns of a view in Exasol, nullable is always NULL,
+        so the expected result needs to be modified. For more reference, see:
+        https://docs.exasol.com/saas/sql_references/system_tables/metadata/exa_all_columns.htm
+        """
+        for key, value_list in expected_multi_output.items():
+            schema, table_or_view = key
+            if not table_or_view.endswith("_v"):
+                continue
+            for column_def in value_list:
+                # Replace nullable: <ANY> with nullable: None
+                column_def["nullable"] = None
+
+    @pytest.mark.xfail(reason=XfailRationale.MANUAL_INDEX.value, strict=True)
     def test_get_indexes(self, connection, use_schema):
         super().test_get_indexes()
 
-    @pytest.mark.xfail(reason=BREAKING_CHANGES_SQL_ALCHEMY_2x, strict=True)
-    def test_get_multi_columns(self):
-        super().test_get_multi_columns()
+    @_multi_combination
+    def test_get_multi_columns(self, get_multi_exp, schema, scope, kind, use_filter):
+        """
+        The default implementation of test_get_multi_columns in
+        class sqlalchemy.testing.suite.ComponentReflectionTest
+        needs to be overridden here, as Exasol always requires nullable to be NULL
+        for the columns of views. The code given in this overriding class
+        method was directly copied. See notes, marked with 'Added', highlighting
+        the changed place.
+        """
+        insp, kws, exp = get_multi_exp(
+            schema,
+            scope,
+            kind,
+            use_filter,
+            Inspector.get_columns,
+            self.exp_columns,
+        )
 
-    @pytest.mark.xfail(reason=BREAKING_CHANGES_SQL_ALCHEMY_2x, strict=True)
-    def test_get_multi_foreign_keys(self):
-        super().test_get_multi_foreign_keys()
+        # Added to convert nullable for columns in views
+        self._convert_view_nullable(exp)
 
-    @pytest.mark.xfail(reason=BREAKING_CHANGES_SQL_ALCHEMY_2x, strict=True)
-    def test_get_multi_pk_constraint(self):
-        super().test_get_multi_pk_constraint()
+        for kw in kws:
+            insp.clear_cache()
+            result = insp.get_multi_columns(**kw)
+            self._check_table_dict(result, exp, self._required_column_keys)
 
-    @pytest.mark.xfail(reason=BREAKING_CHANGES_SQL_ALCHEMY_2x, strict=True)
-    def test_get_view_definition_does_not_exist(self):
-        super().test_get_view_definition_does_not_exist()
+    @_multi_combination
+    def test_get_multi_foreign_keys(
+        self, get_multi_exp, schema, scope, kind, use_filter
+    ):
+        """
+        The default implementation of test_get_multi_foreign_keys in
+        class sqlalchemy.testing.suite.ComponentReflectionTest
+        needs to be overridden here as Exasol does not support custom constraints.
+        The code given in this overriding class method was directly copied. See notes,
+        marked with 'Replaced', highlighting the changed place.
+        """
+
+        def sort_entries_in_place(result_set):
+            for key, value_list in result_set.items():
+                result_set[key] = sorted(value_list, key=lambda x: x["referred_table"])
+
+        insp, kws, exp = get_multi_exp(
+            schema,
+            scope,
+            kind,
+            use_filter,
+            Inspector.get_foreign_keys,
+            self.exp_fks,
+        )
+
+        for kw in kws:
+            insp.clear_cache()
+            result = insp.get_multi_foreign_keys(**kw)
+
+            # Replaced as self._adjust_sort did not work, as some constraints
+            # cannot be added into an Exasol DB as described in define_reflected_tables
+            # self._adjust_sort(
+            #     result, exp, lambda d: tuple(d["referred_table"])
+            # )
+            sort_entries_in_place(exp)
+            sort_entries_in_place(result)
+
+            self._check_table_dict(
+                result,
+                exp,
+                {
+                    "name",
+                    "constrained_columns",
+                    "referred_schema",
+                },
+            )
 
 
 class HasIndexTest(_HasIndexTest):