dstandish commented on a change in pull request #20642: URL: https://github.com/apache/airflow/pull/20642#discussion_r782425545
########## File path: tests/providers/amazon/aws/hooks/test_rds.py ########## @@ -0,0 +1,85 @@ +# +# 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. +# + +import unittest + +import boto3 + +from airflow.providers.amazon.aws.hooks.rds import RDSHook + +try: + from moto import mock_rds +except ImportError: + mock_rds = None + + +class TestRDSHook(unittest.TestCase): + @staticmethod + def _create_instances(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_instance( + DBInstanceIdentifier='mydbinstance1', + DBInstanceClass='db.m4.large', + Engine='postgres', + ) + client.create_db_instance( + DBInstanceIdentifier='mydbinstance2', + DBInstanceClass='db.m4.large', + Engine='mysql', + ) + if not client.describe_db_instances()['DBInstances']: + raise ValueError('AWS not properly mocked') + + @staticmethod + def _create_clusters(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_cluster( + DBClusterIdentifier='my-cluster1', + Engine='postgres', + ) + client.create_db_cluster( + DBClusterIdentifier='my-cluster2', + Engine='mysql', + ) + if not client.describe_db_clusters()['DBClusters']: + raise ValueError('AWS not properly mocked') + + def test_client_attribute_existence(self): + hook = RDSHook(aws_conn_id='aws_default', region_name='us-east-1') + + assert hasattr(hook, 'client') + + @unittest.skipIf(mock_rds is None, 'mock_rds package not present') + @mock_rds + def test_client_db_method_from_aws_rds_hook(self): + self._create_instances() + hook = RDSHook(aws_conn_id='aws_default', region_name='us-east-1') + + instances = hook.client.describe_db_instances()['DBInstances'] + assert len(instances) == 2 + + @unittest.expectedFailure Review comment: why expected failure? what is the purpose of this test? ########## File path: setup.py ########## @@ -194,6 +194,7 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version 'redshift_connector~=2.0.888', 'sqlalchemy_redshift~=0.8.6', pandas_requirement, + 'boto3_stubs[rds]~=1.20.32', Review comment: i think we probably don't need to pin the version here cc @mik-laj @potiuk ########## File path: tests/providers/amazon/aws/hooks/test_rds.py ########## @@ -0,0 +1,85 @@ +# +# 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. +# + +import unittest + +import boto3 + +from airflow.providers.amazon.aws.hooks.rds import RDSHook + +try: + from moto import mock_rds +except ImportError: + mock_rds = None + + +class TestRDSHook(unittest.TestCase): Review comment: should not inherit from unittest anymore we use pytest and sometimes it's incompatible with certain pytest features ########## File path: tests/providers/amazon/aws/hooks/test_rds.py ########## @@ -0,0 +1,85 @@ +# +# 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. +# + +import unittest + +import boto3 + +from airflow.providers.amazon.aws.hooks.rds import RDSHook + +try: + from moto import mock_rds +except ImportError: + mock_rds = None + + +class TestRDSHook(unittest.TestCase): Review comment: since you're not implementing methods anymore, just `conn` maybe it is unnecesssary to test these client methods -- since those are just boto features maybe it would be better to simply test that `hook.conn` returns the right client (namely an RDS client) e.g. i think this works ```python hook = RDSHook() conn = hook.conn assert conn.__class__.__name__ == 'RDS' ``` in trying this out locally i had to patch env var with `AIRFLOW_CONN_AWS_DEFAULT=s3://@` but YMMV i'm updating test util `env_vars` to allow this [here](https://github.com/apache/airflow/pull/20818). then i think we can remove everything related to mock_rds ########## File path: tests/providers/amazon/aws/hooks/test_rds.py ########## @@ -0,0 +1,85 @@ +# +# 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. +# + +import unittest + +import boto3 + +from airflow.providers.amazon.aws.hooks.rds import RDSHook + +try: + from moto import mock_rds +except ImportError: + mock_rds = None + + +class TestRDSHook(unittest.TestCase): + @staticmethod + def _create_instances(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_instance( + DBInstanceIdentifier='mydbinstance1', + DBInstanceClass='db.m4.large', + Engine='postgres', + ) + client.create_db_instance( + DBInstanceIdentifier='mydbinstance2', + DBInstanceClass='db.m4.large', + Engine='mysql', + ) + if not client.describe_db_instances()['DBInstances']: + raise ValueError('AWS not properly mocked') + + @staticmethod + def _create_clusters(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_cluster( + DBClusterIdentifier='my-cluster1', + Engine='postgres', + ) + client.create_db_cluster( + DBClusterIdentifier='my-cluster2', + Engine='mysql', + ) + if not client.describe_db_clusters()['DBClusters']: + raise ValueError('AWS not properly mocked') + + def test_client_attribute_existence(self): + hook = RDSHook(aws_conn_id='aws_default', region_name='us-east-1') + + assert hasattr(hook, 'client') Review comment: think this will fail because no longer implementing this property ########## File path: tests/providers/amazon/aws/hooks/test_rds.py ########## @@ -0,0 +1,85 @@ +# +# 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. +# + +import unittest + +import boto3 + +from airflow.providers.amazon.aws.hooks.rds import RDSHook + +try: + from moto import mock_rds +except ImportError: + mock_rds = None + + +class TestRDSHook(unittest.TestCase): + @staticmethod + def _create_instances(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_instance( + DBInstanceIdentifier='mydbinstance1', + DBInstanceClass='db.m4.large', + Engine='postgres', + ) + client.create_db_instance( + DBInstanceIdentifier='mydbinstance2', + DBInstanceClass='db.m4.large', + Engine='mysql', + ) + if not client.describe_db_instances()['DBInstances']: + raise ValueError('AWS not properly mocked') + + @staticmethod + def _create_clusters(): + client = boto3.client('rds', region_name='us-east-1') + client.create_db_cluster( + DBClusterIdentifier='my-cluster1', + Engine='postgres', + ) + client.create_db_cluster( + DBClusterIdentifier='my-cluster2', + Engine='mysql', + ) + if not client.describe_db_clusters()['DBClusters']: + raise ValueError('AWS not properly mocked') + + def test_client_attribute_existence(self): + hook = RDSHook(aws_conn_id='aws_default', region_name='us-east-1') + + assert hasattr(hook, 'client') + + @unittest.skipIf(mock_rds is None, 'mock_rds package not present') + @mock_rds + def test_client_db_method_from_aws_rds_hook(self): + self._create_instances() + hook = RDSHook(aws_conn_id='aws_default', region_name='us-east-1') + + instances = hook.client.describe_db_instances()['DBInstances'] + assert len(instances) == 2 + + @unittest.expectedFailure + @unittest.skipIf(mock_rds is None, 'mock_rds package not present') Review comment: i think nowadays we use `@pytest.mark.skipif` -- 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]
