revert test_load to before breaking change

Author: jeremydvoss

opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py

@@ -17,10 +17,6 @@
 from unittest.mock import Mock, call, patch
 
 from opentelemetry.instrumentation.auto_instrumentation import _load
-from opentelemetry.instrumentation.dependencies import (
-    DependencyConflict,
-    DependencyConflictError,
-)
 from opentelemetry.instrumentation.environment_variables import (
     OTEL_PYTHON_CONFIGURATOR,
     OTEL_PYTHON_DISABLED_INSTRUMENTATIONS,
@@ -29,7 +25,7 @@
 from opentelemetry.instrumentation.version import __version__
 from opentelemetry.util._importlib_metadata import EntryPoint, entry_points
 
-# TODO: Revert changes from 6d5a5149d02fdec50d8053767b55bc91d80a9228
+
 class TestLoad(TestCase):
     @patch.dict(
         "os.environ", {OTEL_PYTHON_CONFIGURATOR: "custom_configurator2"}
@@ -204,12 +200,6 @@ def test_load_distro_error(self, iter_mock, isinstance_mock):
         # Confirm method raises exception if it fails to load a distro.
         self.assertRaises(Exception, _load._load_distro)
 
-    # @staticmethod
-    # def _instrumentation_failed_to_load_call(entry_point, dependency_conflict):
-    #     return call(
-    #         "Skipping instrumentation %s: %s", entry_point, dependency_conflict
-    #     )
-
     @patch.dict(
         "os.environ",
         {OTEL_PYTHON_DISABLED_INSTRUMENTATIONS: " instr1 , instr3 "},
@@ -277,11 +267,6 @@ def test_load_instrumentors(self, iter_mock, dep_mock):
             [
                 call(ep_mock2, skip_dep_check=True),
                 call(ep_mock4, skip_dep_check=True),
-                # call(ep_mock2, raise_exception_on_conflict=True),
-                # call(ep_mock4, raise_exception_on_conflict=True),
-                # TODO: maybe merge
-                # call(ep_mock2, skip_dep_check=True, raise_exception_on_conflict=True),
-                # call(ep_mock4, skip_dep_check=True, raise_exception_on_conflict=True),
             ]
         )
         self.assertEqual(distro_mock.load_instrumentor.call_count, 2)
@@ -293,11 +278,13 @@ def test_load_instrumentors(self, iter_mock, dep_mock):
         "os.environ",
         {OTEL_PYTHON_DISABLED_INSTRUMENTATIONS: " instr1 , instr3 "},
     )
-    @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
+    )
     @patch(
         "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
     )
-    def test_load_instrumentors_dep_conflict(self, iter_mock, mock_logger):  # pylint: disable=no-self-use
+    def test_load_instrumentors_dep_conflict(self, iter_mock, dep_mock):  # pylint: disable=no-self-use
         ep_mock1 = Mock()
         ep_mock1.name = "instr1"
 
@@ -310,39 +297,27 @@ def test_load_instrumentors_dep_conflict(self, iter_mock, mock_logger):  # pylin
         ep_mock4 = Mock()
         ep_mock4.name = "instr4"
 
-        dependency_conflict = DependencyConflict("1.2.3", None)
-
         distro_mock = Mock()
-        distro_mock.load_instrumentor.side_effect = [
-            None,
-            DependencyConflictError(dependency_conflict),
-        ]
 
-        # If a dependency conflict is raised, that instrumentation should not be loaded, but others still should.
-        # In this case, ep_mock4 will fail to load and ep_mock2 will succeed. (ep_mock1 and ep_mock3 are disabled)
         iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3, ep_mock4)
+        # If a dependency conflict is raised, that instrumentation should not be loaded, but others still should.
+        dep_mock.side_effect = [None, "DependencyConflict"]
         _load._load_instrumentors(distro_mock)
         distro_mock.load_instrumentor.assert_has_calls(
             [
-                call(ep_mock2, raise_exception_on_conflict=True),
-                call(ep_mock4, raise_exception_on_conflict=True),
-            ]
-        )
-        assert distro_mock.load_instrumentor.call_count == 2
-        mock_logger.debug.assert_has_calls(
-            [
-                self._instrumentation_failed_to_load_call(
-                    ep_mock4.name, dependency_conflict
-                )
+                call(ep_mock2, skip_dep_check=True),
             ]
         )
+        distro_mock.load_instrumentor.assert_called_once()
 
-    @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
+    )
     @patch(
         "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
     )
     def test_load_instrumentors_import_error_does_not_stop_everything(
-        self, iter_mock, mock_logger
+        self, iter_mock, dep_mock
     ):
         ep_mock1 = Mock(name="instr1")
         ep_mock2 = Mock(name="instr2")
@@ -356,28 +331,25 @@ def test_load_instrumentors_import_error_does_not_stop_everything(
             (ep_mock1, ep_mock2),
             (),
         ]
+        dep_mock.return_value = None
 
         _load._load_instrumentors(distro_mock)
 
         distro_mock.load_instrumentor.assert_has_calls(
             [
-                call(ep_mock1, raise_exception_on_conflict=True),
-                call(ep_mock2, raise_exception_on_conflict=True),
+                call(ep_mock1, skip_dep_check=True),
+                call(ep_mock2, skip_dep_check=True),
             ]
         )
         self.assertEqual(distro_mock.load_instrumentor.call_count, 2)
-        # TODO: re-enable
-        # mock_logger.exception.assert_any_call(
-        #     "Importing of %s failed, skipping it",
-        #     ep_mock1.name,
-        # )
-
-        # mock_logger.debug.assert_any_call("Instrumented %s", ep_mock2.name)
 
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
+    )
     @patch(
         "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
     )
-    def test_load_instrumentors_raises_exception(self, iter_mock):
+    def test_load_instrumentors_raises_exception(self, iter_mock, dep_mock):
         ep_mock1 = Mock(name="instr1")
         ep_mock2 = Mock(name="instr2")
 
@@ -390,58 +362,18 @@ def test_load_instrumentors_raises_exception(self, iter_mock):
             (ep_mock1, ep_mock2),
             (),
         ]
+        dep_mock.return_value = None
 
         with self.assertRaises(ValueError):
             _load._load_instrumentors(distro_mock)
 
         distro_mock.load_instrumentor.assert_has_calls(
             [
-                call(ep_mock1, raise_exception_on_conflict=True),
+                call(ep_mock1, skip_dep_check=True),
             ]
         )
         self.assertEqual(distro_mock.load_instrumentor.call_count, 1)
 
-    # TODO: re-enable
-    # @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
-    # @patch(
-    #     "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
-    # )
-    # def test_load_instrumentors_module_not_found_error(
-    #     self, iter_mock, mock_logger
-    # ):
-    #     ep_mock1 = Mock()
-    #     ep_mock1.name = "instr1"
-
-    #     ep_mock2 = Mock()
-    #     ep_mock2.name = "instr2"
-
-    #     distro_mock = Mock()
-
-    #     distro_mock.load_instrumentor.side_effect = [
-    #         ModuleNotFoundError("No module named 'fake_module'"),
-    #         None,
-    #     ]
-
-    #     iter_mock.side_effect = [(), (ep_mock1, ep_mock2), ()]
-
-    #     _load._load_instrumentors(distro_mock)
-
-    #     distro_mock.load_instrumentor.assert_has_calls(
-    #         [
-    #             call(ep_mock1, raise_exception_on_conflict=True),
-    #             call(ep_mock2, raise_exception_on_conflict=True),
-    #         ]
-    #     )
-    #     self.assertEqual(distro_mock.load_instrumentor.call_count, 2)
-
-    #     mock_logger.debug.assert_any_call(
-    #         "Skipping instrumentation %s: %s",
-    #         "instr1",
-    #         "No module named 'fake_module'",
-    #     )
-
-    #     mock_logger.debug.assert_any_call("Instrumented %s", ep_mock2.name)
-
     def test_load_instrumentors_no_entry_point_mocks(self):
         distro_mock = Mock()
         _load._load_instrumentors(distro_mock)
@@ -474,4 +406,4 @@ def test_entry_point_dist_finder(self):
             entry_point_without_dist
         )
         # dist are not comparable, being truthy is enough
-        self.assertTrue(new_entry_point_dist)
+        self.assertTrue(new_entry_point_dist)