Addressing @obi1kenobi's PR

Author: jmeulemans

Pipfile

@@ -6,6 +6,8 @@ verify_ssl = true
 [dev-packages]
 bandit = "==1.4.0"
 coveralls = "==1.2.0"
+# The Cython dependency of pymssql is not correctly resolved on all systems
+# so it is explicitly included here.
 cython = "==0.29.2"
 flake8 = "==3.5.0"
 isort = "==4.3.4"

docker-compose.yml

@@ -37,4 +37,4 @@ services:
       - "127.0.0.1:1433:1433"
     environment:
       ACCEPT_EULA: "yes"
-      MSSQL_SA_PASSWORD: Root-secure1 # password requirements are more stringent for MSSQL image
+      MSSQL_SA_PASSWORD: Root-secure1  # password requirements are more stringent for MSSQL image

graphql_compiler/tests/conftest.py

@@ -3,6 +3,7 @@
 import time
 
 import pytest
+import six
 
 from .test_data_tools.data_tool import (
     generate_orient_integration_data, generate_orient_snapshot_data, generate_sql_integration_data,
@@ -61,13 +62,30 @@ def integration_graph_client(request, init_integration_graph_client):
     request.cls.graph_client = init_integration_graph_client
 
 
-@pytest.fixture(scope="class")
+@pytest.fixture(scope='class')
 def sql_integration_data(request):
     """Generate integration data for SQL backends."""
+    # initialize each SQL backend, and open a transaction on each one.
     sql_test_backends = init_sql_integration_test_backends()
+    # write fixture test data within the transaction
     generate_sql_integration_data(sql_test_backends)
+    # make sql backends accessible within the test class
     request.cls.sql_test_backends = sql_test_backends
     # yield the fixture to allow testing class to run
     yield
-    # tear down the fixture after the testing class runs all tests.
+    # tear down the fixture after the testing class runs all tests
+    # including rolling back transaction to ensure all fixture data removed.
     tear_down_integration_test_backends(sql_test_backends)
+
+
+@pytest.fixture(scope='function')
+def sql_integration_test(request):
+    """Open nested transaction before every test function, and rollback transaction afterwards."""
+    sql_test_backends = request.cls.sql_test_backends
+    test_transactions = []
+    for sql_test_backend in six.itervalues(sql_test_backends):
+        transaction = sql_test_backend.connection.begin_nested()
+        test_transactions.append(transaction)
+    yield
+    for transaction in test_transactions:
+        transaction.rollback()

graphql_compiler/tests/integration_tests/integration_backend_config.py

@@ -1,34 +1,34 @@
 # Copyright 2018-present Kensho Technologies, LLC.
 from collections import namedtuple
 
-from ..test_backend import TestBackend
+from .. import test_backend
 
 
 DEFAULT_ROOT_PASSWORD = u'root'
 MSSQL_ROOT_PASSWORD = u'Root-secure1'  # mssql has stricter root password restrictions
 
 SQL_BACKENDS = {
-    TestBackend.POSTGRES,
-    TestBackend.MYSQL,
-    TestBackend.MARIADB,
-    TestBackend.MSSQL,
-    TestBackend.SQLITE,
+    test_backend.POSTGRES,
+    test_backend.MYSQL,
+    test_backend.MARIADB,
+    test_backend.MSSQL,
+    test_backend.SQLITE,
 }
 
 MATCH_BACKENDS = {
-    TestBackend.ORIENTDB,
+    test_backend.ORIENTDB,
 }
 
 SQL_BACKEND_TO_CONNECTION_STRING = {
-    TestBackend.POSTGRES:
+    test_backend.POSTGRES:
         u'postgresql://postgres:{password}@localhost:5432'.format(password=DEFAULT_ROOT_PASSWORD),
-    TestBackend.MYSQL:
+    test_backend.MYSQL:
         u'mysql://root:{password}@127.0.0.1:3306'.format(password=DEFAULT_ROOT_PASSWORD),
-    TestBackend.MARIADB:
+    test_backend.MARIADB:
         u'mysql://root:{password}@127.0.0.1:3307'.format(password=DEFAULT_ROOT_PASSWORD),
-    TestBackend.MSSQL:
+    test_backend.MSSQL:
         u'mssql+pymssql://SA:{password}@localhost:1433'.format(password=MSSQL_ROOT_PASSWORD),
-    TestBackend.SQLITE:
+    test_backend.SQLITE:
         u'sqlite:///:memory:',
 }
 

graphql_compiler/tests/integration_tests/integration_test_helpers.py

@@ -38,20 +38,19 @@ def compile_and_run_match_query(schema, graphql_query, parameters, graph_client)
 def compile_and_run_sql_query(schema, graphql_query, parameters, sql_test_backend):
     """Compiles and runs a SQL query against the supplied SQL backend."""
     # TODO: un-mock the SQL compilation once the SQL backend can run queries.
-    mock_compilation_result = CompilationResult(
-        query=text('SELECT name AS animal_name FROM animal'),
-        language=SQL_LANGUAGE,
-        input_metadata={},
-        output_metadata={'animal_name': OutputMetadata(GraphQLString, False)}
-    )
-
     def mock_sql_compilation(schema, graphql_query, parameters, compiler_metadata):
+        """Mock out SQL backend compilation for unimplemented SQL backend."""
+        mock_compilation_result = CompilationResult(
+            query=text('SELECT name AS animal_name FROM animal'),
+            language=SQL_LANGUAGE,
+            input_metadata={},
+            output_metadata={'animal_name': OutputMetadata(GraphQLString, False)}
+        )
         return mock_compilation_result
 
     compilation_result = mock_sql_compilation(schema, graphql_query, parameters, None)
     query = compilation_result.query
-    results = [
-        dict(result) for result
-        in sql_test_backend.connection.execute(query)
-    ]
+    results = []
+    for result in sql_test_backend.connection.execute(query):
+        results.append(dict(result))
     return results

graphql_compiler/tests/integration_tests/test_backends_integration.py

@@ -15,6 +15,16 @@
 # does not recognize
 # pylint: disable=no-member
 
+
+# Store the typical fixtures required for an integration tests.
+# Individual tests can supply the full @pytest.mark.usefixtures to override if necessary.
+integration_fixture_decorator = pytest.mark.usefixtures(
+    'integration_graph_client',
+    'sql_integration_data',
+    'sql_integration_test',
+)
+
+
 class IntegrationTests(TestCase):
 
     @classmethod
@@ -39,7 +49,7 @@ def compile_and_run_query(cls, graphql_query, parameters):
 
         Args:
             graphql_query: str, GraphQL query string to run against every backend.
-            parameters: Dict, input parameters to the query.
+            parameters: Dict[str, Any], input parameters to the query.
 
         Returns:
             Dict[str, Dict], dictionary mapping the TestBackend to the results fetched from that
@@ -57,7 +67,7 @@ def compile_and_run_query(cls, graphql_query, parameters):
             backend_to_results[backend_name] = results
         return backend_to_results
 
-    @pytest.mark.usefixtures('integration_graph_client', 'sql_integration_data')
+    @integration_fixture_decorator
     def test_backends(self):
         graphql_query = '''
         {

graphql_compiler/tests/test_backend.py

@@ -1,8 +1,8 @@
 # Copyright 2018-present Kensho Technologies, LLC.
-class TestBackend(object):
-    POSTGRES = u'postgresql'
-    MYSQL = u'mysql'
-    MARIADB = u'mariadb'
-    MSSQL = u'mssql'
-    SQLITE = u'sqlite'
-    ORIENTDB = u'orientdb'
+
+POSTGRES = u'postgresql'
+MYSQL = u'mysql'
+MARIADB = u'mariadb'
+MSSQL = u'mssql'
+SQLITE = u'sqlite'
+ORIENTDB = u'orientdb'

graphql_compiler/tests/test_data_tools/data_tool.py

@@ -6,10 +6,10 @@
 import six
 from sqlalchemy import Column, Integer, MetaData, String, Table, create_engine, text
 
+from .. import test_backend
 from ..integration_tests.integration_backend_config import (
     SQL_BACKEND_TO_CONNECTION_STRING, SqlTestBackend
 )
-from ..test_backend import TestBackend
 
 
 def generate_orient_snapshot_data(client):
@@ -46,7 +46,7 @@ def init_sql_integration_test_backends():
     for backend_name, connection_string in six.iteritems(SQL_BACKEND_TO_CONNECTION_STRING):
         engine = create_engine(connection_string)
         # MYSQL and MARIADB do not have a default DB so a DB must be created
-        if backend_name in {TestBackend.MYSQL, TestBackend.MARIADB}:
+        if backend_name in {test_backend.MYSQL, test_backend.MARIADB}:
             # safely create the DB
             engine.execute(text('DROP DATABASE IF EXISTS animals; CREATE DATABASE animals'))
             # update the connection string and engine to connect to this DB specifically
@@ -64,7 +64,7 @@ def init_sql_integration_test_backends():
 
 
 def tear_down_integration_test_backends(sql_test_backends):
-    """Rollback backends' transactions and close the active connections."""
+    """Rollback backends' transactions to wipe test data and to close the active connections."""
     for sql_test_backend in six.itervalues(sql_test_backends):
         sql_test_backend.transaction.rollback()
         sql_test_backend.connection.close()