Propagates more ImportErrors during autodiscovery (#8632)

* Refactors find_related_module tests.

* Narrows exception catching.

* Makes a narrower assertion.

* Cleans up test name.

* Tries to address coverage miss.

* Cleans up comment.

* Fixes typo.

* Adds integration test.

* Fixes bug on ModuleNotFoundError.name when fails early.

* Defaults getattr to None.

celery/loaders/base.py

@@ -253,20 +253,26 @@ def find_related_module(package, related_name):
     # Django 1.7 allows for specifying a class name in INSTALLED_APPS.
     # (Issue #2248).
     try:
+        # Return package itself when no related_name.
         module = importlib.import_module(package)
         if not related_name and module:
             return module
-    except ImportError:
+    except ModuleNotFoundError:
+        # On import error, try to walk package up one level.
         package, _, _ = package.rpartition('.')
         if not package:
             raise
 
     module_name = f'{package}.{related_name}'
 
     try:
+        # Try to find related_name under package.
         return importlib.import_module(module_name)
-    except ImportError as e:
-        import_exc_name = getattr(e, 'name', module_name)
-        if import_exc_name is not None and import_exc_name != module_name:
-            raise e
-        return
+    except ModuleNotFoundError as e:
+        import_exc_name = getattr(e, 'name', None)
+        # If candidate does not exist, then return None.
+        if import_exc_name and module_name.startswith(import_exc_name):
+            return
+
+        # Otherwise, raise because error probably originated from a nested import.
+        raise e

t/integration/test_loader.py

@@ -0,0 +1,22 @@
+from celery import shared_task
+
+
+@shared_task()
+def dummy_task(x, y):
+    return x + y
+
+
+class test_loader:
+    def test_autodiscovery(self, manager):
+        # Arrange
+        expected_package_name, _, module_name = __name__.rpartition('.')
+        unexpected_package_name = 'nonexistent.package.name'
+
+        # Act
+        manager.app.autodiscover_tasks([expected_package_name, unexpected_package_name], module_name, force=True)
+
+        # Assert
+        assert f'{expected_package_name}.{module_name}.dummy_task' in manager.app.tasks
+        assert not any(
+            task.startswith(unexpected_package_name) for task in manager.app.tasks
+        )

t/unit/app/test_loaders.py

@@ -234,19 +234,74 @@ def test_autodiscover_tasks(self):
             base.autodiscover_tasks(['foo'])
             frm.assert_called()
 
-    def test_find_related_module(self):
+    # Happy - get something back
+    def test_find_related_module__when_existent_package_alone(self):
         with patch('importlib.import_module') as imp:
             imp.return_value = Mock()
             imp.return_value.__path__ = 'foo'
-            assert base.find_related_module('bar', 'tasks').__path__ == 'foo'
-            imp.assert_any_call('bar')
-            imp.assert_any_call('bar.tasks')
+            assert base.find_related_module('foo', None).__path__ == 'foo'
+            imp.assert_called_once_with('foo')
 
-            imp.reset_mock()
-            assert base.find_related_module('bar', None).__path__ == 'foo'
-            imp.assert_called_once_with('bar')
+    def test_find_related_module__when_existent_package_and_related_name(self):
+        with patch('importlib.import_module') as imp:
+            first_import = Mock()
+            first_import.__path__ = 'foo'
+            second_import = Mock()
+            second_import.__path__ = 'foo/tasks'
+            imp.side_effect = [first_import, second_import]
+            assert base.find_related_module('foo', 'tasks').__path__ == 'foo/tasks'
+            imp.assert_any_call('foo')
+            imp.assert_any_call('foo.tasks')
+
+    def test_find_related_module__when_existent_package_parent_and_related_name(self):
+        with patch('importlib.import_module') as imp:
+            first_import = ModuleNotFoundError(name='foo.BarApp')  # Ref issue #2248
+            second_import = Mock()
+            second_import.__path__ = 'foo/tasks'
+            imp.side_effect = [first_import, second_import]
+            assert base.find_related_module('foo.BarApp', 'tasks').__path__ == 'foo/tasks'
+            imp.assert_any_call('foo.BarApp')
+            imp.assert_any_call('foo.tasks')
+
+    # Sad - nothing returned
+    def test_find_related_module__when_package_exists_but_related_name_does_not(self):
+        with patch('importlib.import_module') as imp:
+            first_import = Mock()
+            first_import.__path__ = 'foo'
+            second_import = ModuleNotFoundError(name='foo.tasks')
+            imp.side_effect = [first_import, second_import]
+            assert base.find_related_module('foo', 'tasks') is None
+            imp.assert_any_call('foo')
+            imp.assert_any_call('foo.tasks')
+
+    def test_find_related_module__when_existent_package_parent_but_no_related_name(self):
+        with patch('importlib.import_module') as imp:
+            first_import = ModuleNotFoundError(name='foo.bar')
+            second_import = ModuleNotFoundError(name='foo.tasks')
+            imp.side_effect = [first_import, second_import]
+            assert base.find_related_module('foo.bar', 'tasks') is None
+            imp.assert_any_call('foo.bar')
+            imp.assert_any_call('foo.tasks')
+
+    # Sad - errors
+    def test_find_related_module__when_no_package_parent(self):
+        with patch('importlib.import_module') as imp:
+            non_existent_import = ModuleNotFoundError(name='foo')
+            imp.side_effect = non_existent_import
+            with pytest.raises(ModuleNotFoundError) as exc:
+                base.find_related_module('foo', 'tasks')
 
-            imp.side_effect = ImportError()
-            with pytest.raises(ImportError):
-                base.find_related_module('bar', 'tasks')
-            assert base.find_related_module('bar.foo', 'tasks') is None
+            assert exc.value.name == 'foo'
+            imp.assert_called_once_with('foo')
+
+    def test_find_related_module__when_nested_import_missing(self):
+        expected_error = 'dummy import error - e.g. missing nested package'
+        with patch('importlib.import_module') as imp:
+            first_import = Mock()
+            first_import.__path__ = 'foo'
+            second_import = ModuleNotFoundError(expected_error)
+            imp.side_effect = [first_import, second_import]
+            with pytest.raises(ModuleNotFoundError) as exc:
+                base.find_related_module('foo', 'tasks')
+
+            assert exc.value.msg == expected_error