unittest.mock.patch('path') neither forwards or backwards compatible starting from Python >= v3.11

[caronc]

Bug report

Bug description:

Issue

It appears that Python v3.11 and v3.12 are not respecting project functions and classes relative to the module root (module.__init__) when running tests and leveraging unittest.mock.

Proof of Failure

👉 I created a project that can be found here which demonstrates the entire situation.

You can review the import failures across the multiple versions of Python here

Expected Behavior

That the following would work in Python >= 3.11 as it does with all previous versions:

import sys
from unittest import mock


# This actually retrieves apprise.Apprise.Apprise().notify when using
# earlier versions of py.test (hence apprise.__init__.__all__ is respected)
@mock.patch("apprise.Apprise.notify")
def test_py11_and_higher_fails(mock_notify):
    """
    Only works with Python v3.10

    Python v3.11+ produces (where ?? is swapped with 11, or 12 respectively to whichever version of Python is being tested):
    AttributeError: <module 'apprise.Apprise' from '/usr/local/lib/python3.??/site-packages/apprise/Apprise.py'> does not have the attribute 'notify
    """
    pass

For it to work in newer versions of Python, the modules internal object placements are ignored. The following does however work (but incompatible with Python v3.10 and less):

# ...
@mock.patch("apprise.Apprise.Apprise.notify")
def test_py10_and_lower_fails(mock_notify):
     ...

Apprise Background

I'm not sure if this information is relavant; but perhaps the issue is centered around the module itself I'm testing against that makes use of the __all__ setting. Just incase; here are details that surround it:

The __init__ residing in the Apprise Module identifies:

# Get our class Apprise() from within the Apprise.py file
from .Apprise import Apprise

__all__ = [
 'Apprise',  #...
]

• Here is the apprise.__init__.py demonstrating the above

Regular Usage Works

Despite testing failing, all versions of the Python continue to respect import and correctly source the class/function refernced within the modules (inconsisent with unittest). The following works great:

# pip install apprise

# All versions of Python correctly respect module definitions here.
from apprise import Apprise

# Do our magic with it. 
obj = Apprise()

After Thoughts

At first I thought there was an issue with py.test but since Pytest v8.1.1 works in Python v3.10 but not in Python v3.11 or v3.12, I decided to open the ticket here. But the issue seems to be related to unittest.mock.patch and centers around Python v3.11 and higher.

Personally, I feel I've exhausted all avenues with my own research, so hopefully you can help me share what could have been introduced. Any tips/help would be much appreciated. 🙏

CPython versions tested on:

3.6, 3.7, 3.8, 3.9, 3.10, 3.11, 3.12

• bolded are impacted by issue.

Operating systems tested on:

Linux

Linked PRs

• gh-117860: Add tests for resolving names when import rebind names #118176
• [3.12] gh-117860: Add tests for resolving names when import rebind names (GH-118176) #118432

[carljm]

The issue is almost certainly unrelated to __all__; its only runtime effect is on from somemod import * which is not used here.

The issue is related to the fact that using from .Foo import Foo in an __init__.py is inherently bug prone, because it confuses the .Foo submodule with the Foo object within that submodule. Importing a submodule attaches an attribute onto the parent package which is the submodule, but you are also trying to attach an attribute on the parent package which is the Foo object from the submodule. This is a conflict.

Usually this kind of confusion is avoided in a case like this by sticking to the usual lowercase naming convention for modules, so you’d have from .apprise import Apprise instead, and non name collision. My recommendation is if possible you switch to this naming, or same kind of naming that avoids the name collision.

[carljm]

I haven't closed this issue because I think it is worth understanding what actually changed in Python 3.11, so we can consider whether the new semantics are desirable, and if there's anything that should be better tested here to prevent future unintended changes.

Not sure if or when I'll have time to do that investigation.

[caronc]

@carljm I appreciate you getting back; thank you!

I cleaned up my demonstration repository and added python 3.7+ showing the breakage more. It's only for unittest.mock.patch() while direct import logic works perfectly in all versions of Python.

I now understand it has nothing to do with __all__ thanks to your explaination.

While I can understand your bug prone file naming reference, I can't see why the naming of a file should justify all out breakage in newer versions of Python and backwards compatibility loss (test cases only - so far). It's not like the files are prefixed with a period, contain utf-8, aren't syntaxtically correct, or lack a .py extension. All files can be successfully compiled into bytecode without problem as well. The can also all be seamlessly sourced correctly through import calls. This should be enough to justify a unique namespace reguardless of the filename?

The effort level in renaming all of my project (Apprise) files to lowercase will take some time (and testing) and will likely introduce breaking changes to those using it. I'll let you know if that helps 👍 .

I do appreciate you leaving the ticket open pending more investigation; I hope something gets uncovered by one who does have time to look into it further 🙏 .

[serhiy-storchaka]

bpo-44686/gh-88852 made the unittest.mock module use pkgutil.resolve_name() .

There is a subtle difference in the behavior. Both start with importing the first component of the dotted path, then resolve the following components relatively to the parent object, but

• pkgutil.resolve_name() first try to import the module with the {parent}.{name} name, and only if the import fails, it look up the name attribute in the parent module.
• The older unittest.mock code first tried to look up the name attribute in the parent module, and only if it was not found, it tried to import the module with the {parent}.{name} name.

It is difficult to say what way is more correct. The solution -- avoid ambiguous situations in which they give different results.

[caronc]

@serhiy-storchaka awesome research!

But would breaking backwards compatibility not imply it was more correct before and the proposed change you described would be better handled during a Python v4 (breaking as it is)?

[carljm]

Hmm, I don't know the background of why pkgutil.resolve_name works the way it does, but it seems to me that pkgutil.resolve_name is buggy here, because its priority ordering of a package attribute vs submodule is backwards from that of Python's own import behavior. I don't know why this difference would be desirable.

[serhiy-storchaka]

What is the Python's own import behavior?

If there is a package a with two submodules b and c, and a/__init__.py containing from . import c as b, then we get different results for resolving a.b.__name__, depending on the way how to import:

$ ./python -c 'import a.b; print(a.b.__name__)'
a.b
$ ./python -c 'from a import b; print(b.__name__)'
a.c
$ ./python -c 'from a.b import __name__; print(__name__)'
a.b

[carljm]

Heh, good point :) I'd forgotten that Python itself is so inconsistent in such cases. This really suggests that "don't do this" is the only advisable course.

[caronc]

... "don't do this" is the only advisable course.

Personally it just demonstrates the issue is much larger. It's also apples to oranges in the sense that the way imports behave have no breaking changes between minor releases.

It also steps outside of the scope of the unittest which has a breaking change on it.Not then a 2-3 release depreciation phase or a release not to justify it (I'm hoping to be enlightened here 🙂🙏)?

[carljm]

I agree that the change in Python 3.11 was a breaking change, and I suspect that had that been recognized at the time, the change would not have been made.

But the breakage appears to have been rare in practice, since it's been 18 months since the release of Python 3.11, and this is the first it's been raised. And the breakage only impacts a usage that is both unusual and already inherently unreliable (thus already discouraged), given Python's inconsistency in whether a sub module or a package attribute is prioritized.

At this point there are already two Python feature releases out with the new behavior, and one of them is already into security-fix-only mode. So it is just too late for the "it was a back-compat break" argument to be relevant to what we do now. It was a back-compat break, and that's unfortunate. But we aren't going to compound that error by intentionally making another back-compat break now in 3.13, in order to revert to a behavior that doesn't appear to be inherently any better. And we certainly aren't going to backport a breaking change to Python 3.11 or 3.12, which are in security-fix-only and bug fix-only maintenance.

If the goal is to minimize breaking changes, the status quo is now the best option available.

[serhiy-storchaka]

It may be not late to change the behavior if it is obviously wrong. The case is so obscure, that it affects not many people. But it is not obvious to me what variant is more right. I tried to change the behavior of pkgutil.resolve_name() -- and all tests are still passed.

It happens when a.b differs from sys.modules['a.b']. The workaround is to synchronize them: sys.modules['a.b'] = a.b. Perhaps.

[serhiy-storchaka]

In case of apprise, import apprise.Apprise as Apprise and from apprise import Apprise give different results. Actually, simple addition of import apprise.Apprise in the user code may ruin this package. I think this a sign of problems in such code.