The fix for bpo-17636 added support for falling back to sys.modules when a module isn't directly present on the module. But mock doesn't have the same behavior - it'll try the import, and then try to get the value off the object. If it's not there it just errors out.

Instead it should also consult sys.modules to be consistent with import semantics.

What's the real world use case for this?

It's related to bpo-39336. If you have an immutable package which doesn't allow it's children to be published on it when following the chain of packages it ends up not arriving at the final module.

But you can also hit it if you end up doing the patch during a relative import, although that seems much less likely. But it generally seems matching the behavior of imports would be good to me.

I'm afraid I don't understand "immutable package which doesn't allow it's children to be published on it", can you give an example?

My actual scenario involves a custom module loader where the modules are published are completely immutable (it ends up publishing an object which isn't a subtype of module). It can still have normal Python modules as a child which aren't immutable, so they could still be patched by Mock (or it could have immutable sub-packages which Mock wouldn't be able to patch).

So imagine something like this:

immutable_package\init.py
__immutable__ = True
x = 2

immutable_package\x.py
y = 2

Doing a "from immutable_package import x" would normally publish "x" as a child onto the package. But because the package is immutable, this is impossible, and the assignment is ignored with a warning.

When Mock gets a call to patch on something like "immutable_package.x.y", it's not going to find x, even though if I were to write "from immutable_package.x import y" or "from immutable_package import x" it would succeed.

Cases can be contrived without all of this though where the child isn't published on it's parent, but it requires

x/init.py
from x.pkg import child

x/pkg/init.py:
x = 1

x/pkg/child.py:
from unittest.mock import patch
y = 42

@patch('x.pkg.child.y', 100)
def f():
print(y)

f()

"python -m x" will fail without the patch but succeed with it.

Apologies, but I'm still not sure what "the modules are published" means?
"publish "x" as a child onto the package" also doesn't mean much to me, I'm afraid. Are you aware of any importlib docs or some such which might be able to explain this to me?

When "It can still have normal Python modules as a child which aren't immutable", what happens when you try to patch the immutable module? What happens if you try to patch the mutable module below it?

When "the assignment is ignored with a warning.", what is doing that ignoring?

Unless I'm missing something, this feels like such an edge case I'm not sure mock.patch should be trying to support it. Should this be something that is handled by your immutable import thing?

Sorry, publish may not necessarily be the best term. When you do "from foo import bar" and foo is a package Python will set the bar module onto the foo module. That leads to the common mistake of doing "import foo" and then "foo.bar.baz" and later removing the thing that do "from foo import bar" and having things blow up later.

Without additional support there's no way to patch the immutable module. We can provide a mode where we enable the patching for testing and disable it in a production environment though. That basically just involves passing a proxy object down to Mock. And monkey patching the mutable module is perfectly fine.

The thing that's doing the ignoring of the assignment is the import system. So it's now okay if the package raises an AttributeError.

There's not really a great way to work around this other than just bypassing mock's resolution of the object here - i.e. replacing mock.patch along with _get_target, _importer, and _dot_lookup and calling mock._patch directly, which isn't very nice (but is do-able).

And while this is a strange way to arrive at a module existing in sys.modules but not being on the package it is something that can happen in the normal course of imports, hence the reason why the import system handles this by checking sys.modules today. It's also just turning what is currently an error while mocking into a success case with a simple few line change.

Hmm, the more we get into this, the less comfortable I am of the patch in the PR.

Instead of copying and pasting more code that's likely to get out of sync, could you change the PR to just replace _importer with the correct parts of importlib to support your usecase?

Keep the unit test though, useful to show in mock why the change is needed, and make sure we do something sensible in the backport!

I like that idea, let me see if I can find a way to do that. This is a little bit different in that it's implicitly trying to find a module, and supports dotting through non-packages as well, but maybe there's a way to leverage importlib and still keep the existing behavior.