Raise TypeError on bare decorator factory usage, fix existing bare usages, and add regression tests (aws#10131)

Author: Alan4506

tests/functional/s3/test_mv_command.py

@@ -835,7 +835,7 @@ def test_mv_works_if_outpost_access_point_arn_resolves_to_different_bucket(
         self.assertEqual(self.operations_called[2][0].name, 'CopyObject')
         self.assertEqual(self.operations_called[3][0].name, 'DeleteObject')
 
-    @requires_crt
+    @requires_crt()
     def test_mv_works_if_mrap_arn_resolves_to_different_bucket(self):
         cmdline = (
             f"{self.prefix} s3://bucket/key "
@@ -911,7 +911,7 @@ def test_skips_validation_if_keys_are_different_outpost_alias(self):
         )
         self.assert_runs_mv_without_validation(cmdline)
 
-    @requires_crt
+    @requires_crt()
     def test_skips_validation_if_keys_are_different_mrap_arn(self):
         cmdline = (
             f"{self.prefix} s3://bucket/key "

tests/functional/s3transfer/test_crt.py

@@ -62,7 +62,7 @@ def on_done(self, future, **kwargs):
         self.on_done_future = future
 
 
-@requires_crt
+@requires_crt()
 class TestCRTTransferManager(unittest.TestCase):
     def setUp(self):
         self.region = 'us-west-2'

tests/integration/s3transfer/test_crt.py

@@ -49,7 +49,7 @@ def on_done(self, **kwargs):
         self.on_done_called = True
 
 
-@requires_crt
+@requires_crt()
 class TestCRTS3Transfers(BaseTransferManagerIntegTest):
     """Tests for the high level s3transfer based on CRT implementation."""
 

tests/unit/s3transfer/test_crt.py

@@ -89,7 +89,7 @@ def test_multiple_acquires_return_same_lock(self, mock_crt_process_lock):
         mock_crt_process_lock.return_value.acquire.assert_called_once_with()
 
 
-@requires_crt
+@requires_crt()
 class TestBotocoreCRTRequestSerializer(unittest.TestCase):
     def setUp(self):
         self.region = 'us-west-2'
@@ -291,7 +291,7 @@ def test_to_crt_credentials_provider(self, botocore_credentials):
         self.assert_crt_credentials(crt_credentials)
 
 
-@requires_crt
+@requires_crt()
 class TestCRTTransferFuture(unittest.TestCase):
     def setUp(self):
         self.mock_s3_request = mock.Mock(awscrt.s3.S3RequestType)
@@ -320,7 +320,7 @@ def test_set_exception_can_override_previous_exception(self):
             self.future.result()
 
 
-@requires_crt
+@requires_crt()
 class TestOnBodyFileObjWriter(unittest.TestCase):
     def test_call(self):
         fileobj = io.BytesIO()

tests/unit/test_decorators.py

@@ -0,0 +1,115 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed 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.
+from unittest import mock
+
+import pytest
+
+from tests.utils.s3transfer import (
+    requires_crt,
+    skip_if_using_serial_implementation,
+    skip_if_windows,
+)
+
+
+class TestSkipIfWindows:
+    def test_bare_skip_if_windows_fails_immediately(self):
+        with pytest.raises(TypeError):
+
+            @skip_if_windows
+            def my_test():
+                pass
+
+    def test_skip_if_windows_skips_on_windows(self):
+        with mock.patch('tests.utils.s3transfer.platform') as mock_platform:
+            mock_platform.system.return_value = 'Windows'
+
+            @skip_if_windows('Not supported on Windows')
+            def my_test():
+                assert False
+
+            assert getattr(my_test, '__unittest_skip__', False) is True
+
+    def test_skip_if_windows_runs_on_non_windows(self):
+        with mock.patch('tests.utils.s3transfer.platform') as mock_platform:
+            mock_platform.system.return_value = 'Linux'
+
+            @skip_if_windows('Not supported on Windows')
+            def my_test():
+                pass
+
+            assert getattr(my_test, '__unittest_skip__', False) is False
+
+
+class TestSkipIfUsingSerialImplementation:
+    def test_bare_skip_if_using_serial_implementation_fails_immediately(self):
+        with pytest.raises(TypeError):
+
+            @skip_if_using_serial_implementation
+            def my_test():
+                pass
+
+    def test_skip_if_using_serial_implementation_skips_when_serial(self):
+        with mock.patch(
+            'tests.utils.s3transfer.is_serial_implementation',
+            return_value=True,
+        ):
+
+            @skip_if_using_serial_implementation(
+                'Not supported in serial mode'
+            )
+            def my_test():
+                assert False
+
+            assert getattr(my_test, '__unittest_skip__', False) is True
+
+    def test_skip_if_using_serial_implementation_runs_when_not_serial(self):
+        with mock.patch(
+            'tests.utils.s3transfer.is_serial_implementation',
+            return_value=False,
+        ):
+
+            @skip_if_using_serial_implementation(
+                'Not supported in serial mode'
+            )
+            def my_test():
+                pass
+
+            assert getattr(my_test, '__unittest_skip__', False) is False
+
+
+class TestRequiresCrt:
+    def test_bare_requires_crt_fails_immediately(self):
+        with pytest.raises(TypeError):
+
+            @requires_crt
+            def my_test():
+                pass
+
+    def test_requires_crt_skips_when_no_crt(self):
+        with mock.patch('tests.utils.s3transfer.HAS_CRT', False):
+
+            @requires_crt()
+            def my_test():
+                assert False
+
+            assert getattr(my_test, '__unittest_skip__', False) is True
+
+    def test_requires_crt_runs_when_crt_available(self):
+        with mock.patch('tests.utils.s3transfer.HAS_CRT', True):
+
+            @requires_crt()
+            def my_test():
+                pass
+
+            assert getattr(my_test, '__unittest_skip__', False) is False

tests/utils/s3transfer/__init__.py

@@ -99,6 +99,11 @@ def test_some_non_windows_stuff(self):
             self.assertEqual(...)
 
     """
+    if callable(reason):
+        raise TypeError(
+            "Use @skip_if_windows('reason') with parentheses, "
+            "not bare @skip_if_windows"
+        )
 
     def decorator(func):
         return unittest.skipIf(
@@ -110,17 +115,30 @@ def decorator(func):
 
 def skip_if_using_serial_implementation(reason):
     """Decorator to skip tests when running as the serial implementation"""
+    if callable(reason):
+        raise TypeError(
+            "Use @skip_if_using_serial_implementation('reason') with parentheses, "
+            "not bare @skip_if_using_serial_implementation"
+        )
 
     def decorator(func):
         return unittest.skipIf(is_serial_implementation(), reason)(func)
 
     return decorator
 
 
-def requires_crt(cls, reason=None):
+def requires_crt(reason=None):
+    if callable(reason):
+        raise TypeError(
+            "Use @requires_crt() with parentheses, not bare @requires_crt"
+        )
     if reason is None:
         reason = "Test requires awscrt to be installed."
-    return unittest.skipIf(not HAS_CRT, reason)(cls)
+
+    def decorator(func):
+        return unittest.skipIf(not HAS_CRT, reason)(func)
+
+    return decorator
 
 
 class StreamWithError: