No DeprecationWarning deduping during testing

cirq-core/cirq/_compat.py

@@ -91,10 +91,25 @@ def proper_eq(a: Any, b: Any) -> bool:
 def _warn_or_error(msg):
     from cirq.testing.deprecation import ALLOW_DEPRECATION_IN_TEST
 
-    called_from_test = 'CIRQ_TESTING' in os.environ
     deprecation_allowed = ALLOW_DEPRECATION_IN_TEST in os.environ
-    if called_from_test and not deprecation_allowed:
-        raise ValueError(f"Cirq should not use deprecated functionality: {msg}")
+    if _called_from_test() and not deprecation_allowed:
+        for filename, line_number, function_name, text in reversed(traceback.extract_stack()):
+            if (
+                not _is_internal(filename)
+                and "_compat.py" not in filename
+                and "_test.py" in filename
+            ):
+                break
+        raise ValueError(
+            f"During testing using Cirq deprecated functionality is not allowed: {msg}"
+            f"Update to non-deprecated functionality, or alternatively, you can quiet "
+            f"this error by removing the CIRQ_TESTING environment variable "
+            f"temporarily with `@mock.patch.dict(os.environ, clear='CIRQ_TESTING')`.\n"
+            f"In case the usage of deprecated cirq is intentional, use "
+            f"`with cirq.testing.assert_deprecated(...):` around this line:\n"
+            f"{filename}:{line_number}: in {function_name}\n"
+            f"\t{text}"
+        )
 
     # we have to dynamically count the non-internal frames
     # due to the potentially multiple nested module wrappers
@@ -396,8 +411,22 @@ def _is_internal(filename: str) -> bool:
 _warned: Set[str] = set()
 
 
-def _deduped_module_warn_or_error(old_module_name, new_module_name, deadline):
-    if old_module_name in _warned:
+def _called_from_test() -> bool:
+    return 'CIRQ_TESTING' in os.environ
+
+
+def _should_dedupe_module_deprecation() -> bool:
+    """Whether module deprecation warnings should be deduped or not.
+
+    We should always dedupe when not called from test.
+    We should only dedupe during tests if forced.
+    """
+    force_dedupe = "CIRQ_FORCE_DEDUPE_MODULE_DEPRECATION" in os.environ
+    return not _called_from_test() or force_dedupe
+
+
+def _deduped_module_warn_or_error(old_module_name: str, new_module_name: str, deadline: str):
+    if _should_dedupe_module_deprecation() and old_module_name in _warned:
         return
 
     _warned.add(old_module_name)

cirq-core/cirq/_compat_test.py

@@ -14,13 +14,15 @@
 import importlib
 import logging
 import multiprocessing
+import os
 import sys
 import traceback
 import types
 import warnings
 from types import ModuleType
 from typing import Callable, Optional
 from importlib.machinery import ModuleSpec
+from unittest import mock
 
 import numpy as np
 import pandas as pd
@@ -125,7 +127,9 @@ def old_func(*args, **kwargs):
     ):
         assert old_func(1, 2) == 3
 
-    with pytest.raises(ValueError, match='Cirq should not use deprecated functionality'):
+    with pytest.raises(
+        ValueError, match='During testing using Cirq deprecated functionality is not allowed'
+    ):
         old_func(1, 2)
 
     with pytest.raises(AssertionError, match='deadline should match vX.Y'):
@@ -168,7 +172,9 @@ def f(new_count):
         # pylint: enable=no-value-for-parameter
         # pylint: enable=unexpected-keyword-arg
 
-    with pytest.raises(ValueError, match='Cirq should not use deprecated functionality'):
+    with pytest.raises(
+        ValueError, match='During testing using Cirq deprecated functionality is not allowed'
+    ):
         # pylint: disable=unexpected-keyword-arg
         # pylint: disable=no-value-for-parameter
         f(double_count=1)
@@ -223,7 +229,9 @@ def test_wrap_module():
     ):
         _ = wrapped.foo
 
-    with pytest.raises(ValueError, match='Cirq should not use deprecated functionality'):
+    with pytest.raises(
+        ValueError, match='During testing using Cirq deprecated functionality is ' 'not allowed'
+    ):
         _ = wrapped.foo
 
     with cirq.testing.assert_logs(count=0):
@@ -264,7 +272,9 @@ class OldClass(NewClass):
         assert repr(old_obj) == 'NewClass: 1'
         assert 'OldClass' in old_obj.hello()
 
-    with pytest.raises(ValueError, match='Cirq should not use deprecated functionality'):
+    with pytest.raises(
+        ValueError, match='During testing using Cirq deprecated functionality is not allowed'
+    ):
         OldClass('1')
 
     with pytest.raises(AssertionError, match='deadline should match vX.Y'):
@@ -516,6 +526,7 @@ def isolated_func(*args, **kwargs):
     return isolated_func
 
 
+@mock.patch.dict(os.environ, {"CIRQ_FORCE_DEDUPE_MODULE_DEPRECATION": "1"})
 @pytest.mark.parametrize(
     'outdated_method,deprecation_messages',
     [

cirq-core/cirq/protocols/json_serialization_test.py

@@ -12,14 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import contextlib
-
+import dataclasses
 import datetime
-import importlib
 import io
 import json
 import os
 import pathlib
+import warnings
 from typing import Dict, List, Optional, Tuple
+from unittest import mock
 
 import numpy as np
 import pandas as pd
@@ -33,18 +34,42 @@
 from cirq.testing.json import ModuleJsonTestSpec, spec_for
 
 REPO_ROOT = pathlib.Path(__file__).parent.parent.parent.parent
-TESTED_MODULES = [
-    'cirq_aqt',
-    'cirq_ionq',
-    'cirq_google',
-    'cirq.protocols',
-    'non_existent_should_be_fine',
-]
+
+
+@dataclasses.dataclass
+class _ModuleDeprecation:
+    old_name: str
+    deprecation_assertion: contextlib.AbstractContextManager
+
+
+# tested modules and their deprecation settings
+TESTED_MODULES: Dict[str, Optional[_ModuleDeprecation]] = {
+    'cirq_aqt': _ModuleDeprecation(
+        old_name="cirq.aqt",
+        deprecation_assertion=cirq.testing.assert_deprecated(
+            "cirq.aqt", deadline="v0.14", count=None
+        ),
+    ),
+    'cirq_ionq': _ModuleDeprecation(
+        old_name="cirq.ionq",
+        deprecation_assertion=cirq.testing.assert_deprecated(
+            "cirq.ionq", deadline="v0.14", count=None
+        ),
+    ),
+    'cirq_google': _ModuleDeprecation(
+        old_name="cirq.google",
+        deprecation_assertion=cirq.testing.assert_deprecated(
+            "cirq.google", deadline="v0.14", count=None
+        ),
+    ),
+    'cirq.protocols': None,
+    'non_existent_should_be_fine': None,
+}
 
 
 def _get_testspecs_for_modules():
     modules = []
-    for m in TESTED_MODULES:
+    for m in TESTED_MODULES.keys():
         try:
             modules.append(spec_for(m))
         except ModuleNotFoundError:
@@ -165,7 +190,11 @@ def test_fail_to_resolve():
 ### MODULE CONSISTENCY tests
 
 
-@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS)
+@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS, ids=repr)
+# during test setup deprecated submodules are inspected and trigger the
+# deprecation error in testing. It is cleaner to just turn it off then to assert
+# deprecation for each submodule.
+@mock.patch.dict(os.environ, clear='CIRQ_TESTING')
 def test_shouldnt_be_serialized_no_superfluous(mod_spec: ModuleJsonTestSpec):
     # everything in the list should be ignored for a reason
     names = set(mod_spec.get_all_names())
@@ -177,7 +206,11 @@ def test_shouldnt_be_serialized_no_superfluous(mod_spec: ModuleJsonTestSpec):
     )
 
 
-@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS)
+@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS, ids=repr)
+# during test setup deprecated submodules are inspected and trigger the
+# deprecation error in testing. It is cleaner to just turn it off then to assert
+# deprecation for each submodule.
+@mock.patch.dict(os.environ, clear='CIRQ_TESTING')
 def test_not_yet_serializable_no_superfluous(mod_spec: ModuleJsonTestSpec):
     # everything in the list should be ignored for a reason
     names = set(mod_spec.get_all_names())
@@ -187,7 +220,7 @@ def test_not_yet_serializable_no_superfluous(mod_spec: ModuleJsonTestSpec):
     )
 
 
-@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS)
+@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS, ids=repr)
 def test_mutually_exclusive_blacklist(mod_spec: ModuleJsonTestSpec):
     common = set(mod_spec.should_not_be_serialized) & set(mod_spec.not_yet_serializable)
     assert len(common) == 0, (
@@ -196,7 +229,7 @@ def test_mutually_exclusive_blacklist(mod_spec: ModuleJsonTestSpec):
     )
 
 
-@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS)
+@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS, ids=repr)
 def test_resolver_cache_vs_should_not_serialize(mod_spec: ModuleJsonTestSpec):
     resolver_cache_types = set([n for (n, _) in mod_spec.get_resolver_cache_types()])
     common = set(mod_spec.should_not_be_serialized) & resolver_cache_types
@@ -208,7 +241,7 @@ def test_resolver_cache_vs_should_not_serialize(mod_spec: ModuleJsonTestSpec):
     )
 
 
-@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS)
+@pytest.mark.parametrize('mod_spec', MODULE_TEST_SPECS, ids=repr)
 def test_resolver_cache_vs_not_yet_serializable(mod_spec: ModuleJsonTestSpec):
     resolver_cache_types = set([n for (n, _) in mod_spec.get_resolver_cache_types()])
     common = set(mod_spec.not_yet_serializable) & resolver_cache_types
@@ -414,15 +447,14 @@ def test_internal_serializer_types():
         _ = json_serialization._ContextualSerialization._from_json_dict_(**serialization_json)
 
 
+# during test setup deprecated submodules are inspected and trigger the
+# deprecation error in testing. It is cleaner to just turn it off then to assert
+# deprecation for each submodule.
+@mock.patch.dict(os.environ, clear='CIRQ_TESTING')
 def _list_public_classes_for_tested_modules():
-    cirq_google_on_path = importlib.util.find_spec("cirq_google") is not None
-
-    ctx_manager = (
-        cirq.testing.assert_deprecated("cirq.google", deadline="v0.14")
-        if cirq_google_on_path
-        else contextlib.suppress()
-    )
-    with ctx_manager:
+    # to remove DeprecationWarning noise during test collection
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
         return [
             (mod_spec, o, n)
             for mod_spec in MODULE_TEST_SPECS
@@ -538,32 +570,42 @@ def test_to_from_json_gzip():
 
 
 def _eval_repr_data_file(path: pathlib.Path, deprecation_deadline: Optional[str]):
-    ctx_manager = (
-        cirq.testing.assert_deprecated(deadline=deprecation_deadline, count=None)
-        if deprecation_deadline
-        else contextlib.suppress()
-    )
-    with ctx_manager:
-        imports = {
-            'cirq': cirq,
-            'datetime': datetime,
-            'pd': pd,
-            'sympy': sympy,
-            'np': np,
-            'datetime': datetime,
-        }
-        try:
-            import cirq_google
+    content = path.read_text()
+    ctx_managers: List[contextlib.AbstractContextManager] = [contextlib.suppress()]
+    if deprecation_deadline:
+        # we ignore coverage here, because sometimes there are no deprecations at all in any of the
+        # modules
+        # coverage: ignore
+        ctx_managers = [cirq.testing.assert_deprecated(deadline=deprecation_deadline, count=None)]
+
+    for deprecation in TESTED_MODULES.values():
+        if deprecation is not None and deprecation.old_name in content:
+            ctx_managers.append(deprecation.deprecation_assertion)
+
+    imports = {
+        'cirq': cirq,
+        'datetime': datetime,
+        'pd': pd,
+        'sympy': sympy,
+        'np': np,
+        'datetime': datetime,
+    }
+    try:
+        import cirq_google
 
-            imports['cirq_google'] = cirq_google
-        except ImportError:
-            pass
+        imports['cirq_google'] = cirq_google
+    except ImportError:
+        pass
+
+    with contextlib.ExitStack() as stack:
+        for ctx_manager in ctx_managers:
+            stack.enter_context(ctx_manager)
         obj = eval(
-            path.read_text(),
+            content,
             imports,
             {},
         )
-    return obj
+        return obj
 
 
 def assert_repr_and_json_test_data_agree(

cirq-core/cirq/testing/deprecation.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 import logging
 import os
-from contextlib import contextmanager
 from typing import Optional
 
 from cirq._compat import deprecated_parameter
@@ -22,7 +21,6 @@
 ALLOW_DEPRECATION_IN_TEST = 'ALLOW_DEPRECATION_IN_TEST'
 
 
-@contextmanager
 @deprecated_parameter(
     deadline='v0.12',
     fix='Use count instead.',
@@ -51,27 +49,28 @@ def assert_deprecated(*msgs: str, deadline: str, count: Optional[int] = 1):
             messages have to equal count.
     """
 
-    orig_exist, orig_value = (
-        ALLOW_DEPRECATION_IN_TEST in os.environ,
-        os.environ.get(ALLOW_DEPRECATION_IN_TEST, None),
-    )
-    os.environ[ALLOW_DEPRECATION_IN_TEST] = 'True'
-    try:
-        with assert_logs(
-            *(msgs + (deadline,)),
-            min_level=logging.WARNING,
-            max_level=logging.WARNING,
-            count=count,
-        ):
-            yield True
-    finally:
-        try:
-            if orig_exist:
+    class DeprecationAssertContext:
+        def __enter__(self):
+            self.orig_exist, self.orig_value = (
+                ALLOW_DEPRECATION_IN_TEST in os.environ,
+                os.environ.get(ALLOW_DEPRECATION_IN_TEST, None),
+            )
+            os.environ[ALLOW_DEPRECATION_IN_TEST] = 'True'
+            self.assert_logs = assert_logs(
+                *(msgs + (deadline,)),
+                min_level=logging.WARNING,
+                max_level=logging.WARNING,
+                count=count,
+            )
+            self.assert_logs.__enter__()
+
+        def __exit__(self, exc_type, exc_val, exc_tb):
+            if self.orig_exist:
                 # mypy can't resolve that orig_exist ensures that orig_value
                 # of type Optional[str] can't be None
-                os.environ[ALLOW_DEPRECATION_IN_TEST] = orig_value  # type: ignore
+                os.environ[ALLOW_DEPRECATION_IN_TEST] = self.orig_value  # type: ignore
             else:
                 del os.environ[ALLOW_DEPRECATION_IN_TEST]
-        except:
-            # this is only for nested deprecation checks
-            pass
+            self.assert_logs.__exit__(exc_type, exc_val, exc_tb)
+
+    return DeprecationAssertContext()