PR: Fix some tests failing on Python 3.12

[juliangilbey]

Description of Changes

• Wrote at least one-line docstrings (for any new functions)
• Added unit test(s) covering the changes (if testable)

• Included a screenshot or animation (if affecting the UI, see Licecap)

These fix all of the issues identified in #21688 except for test_move_to_first_breakpoint[False]. The tests using called_with were both broken, and this patch also fixes this.

Issue(s) Resolved

Almost resolves (fixes) #21688, but not quite

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @juliangilbey

[pep8speaks]

Hello @juliangilbey! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file spyder/plugins/editor/widgets/tests/test_warnings.py:

Line 269:80: E501 line too long (84 > 79 characters)

Comment last updated at 2024-01-12 06:38:56 UTC

[ccordoba12]

Thanks @juliangilbey for your help with this! I left a small comment for you. The rest looks good to me.

[ccordoba12]

This is clearly not a fix for this test because we need to check that all files have been renamed. And that's detected by our Windows tests.

So, I recommend instead to revert these changes and simply skip it in your test env.

[juliangilbey]

Actually, this test didn't do anything (even on Windows), because called_once always silently returned a true value without testing anything. I've just pushed a new version of the patch, replacing the assert_called_with() call, which only tests the most recent call, with assert_any_call(), which checks whether the method has ever been called with the specified arguments. So this new version of the patch now correctly checks for all three calls.

[juliangilbey]

Small clarification to that comment: called_once() always silently returned a true value on Python <= 3.11. But on Python 3.12, it throws an error, which is how I discovered this bug.

[juliangilbey]

It turns out that fixing this test shows that renamed_tree completely fails on Windows. Here's the relevant part of the Windows error message:

self = <MagicMock name='renamed' id='1968684230640'>, args = ()
kwargs = {'dest': '/test/dir/file1.py', 'source': '/test/directory/file1.py'}
expected = call(source='/test/directory/file1.py', dest='/test/dir/file1.py')
cause = None
actual = [call(source='/test/directory/file1.py', dest='/test/directory/file1.py'), call(source='/test/directory/file2.txt', dest='/test/directory/file2.txt'), call(source='/test/directory/file4.rst', dest='/test/directory/file4.rst')]
expected_string = "renamed(source='/test/directory/file1.py', dest='/test/dir/file1.py')"

    def assert_any_call(self, /, *args, **kwargs):
        """assert the mock has been called with the specified arguments.
    
        The assert passes if the mock has *ever* been called, unlike
        `assert_called_with` and `assert_called_once_with` that only pass if
        the call is the most recent one."""
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        cause = expected if isinstance(expected, Exception) else None
        actual = [self._call_matcher(c) for c in self.call_args_list]
        if cause or expected not in _AnyComparer(actual):
            expected_string = self._format_mock_call_signature(args, kwargs)
>           raise AssertionError(
                '%s call not found' % expected_string
            ) from cause
E           AssertionError: renamed(source='/test/directory/file1.py', dest='/test/dir/file1.py') call not found

Note the key information here: the actual variable contains 3 calls such as:
call(source='/test/directory/file1.py', dest='/test/directory/file1.py')
showing that renamed() was called with the source directory name twice.

Let's run the steps of renamed_tree() manually in a Python 3.9 session on Windows; here is what happens:

>>> import os
>>> import os.path as osp
>>> os.getcwd()
'C:\\Program Files\\Python39'
>>> source = "/test/directory"
>>> dest = "/test/dir"
>>> fname = "/test/directory/file1.py"
>>> dirname = osp.abspath(source)
>>> dirname
'C:\\test\\directory'
>>> osp.abspath(fname)
'C:\\test\\directory\\file1.py'
>>> osp.abspath(fname).startswith(dirname)
True
>>> fname.replace(dirname, dest)
'/test/directory/file1.py'

Oops! fname contains the POSIX form of the path name, /test/directory/file1.py. But dirname contains the Windows form, C:\\test\\directory. Therefore the fname.replace() call doesn't actually replace anything.

The next question, then, is how to fix renamed_tree(). It certainly shouldn't be using dirname in the replace() call. The simplest fix would be to use fname.replace(to_text_string(source), tofile) instead. This does run a small risk, though, of replacing multiple occurrences of the same string within the source. So perhaps better would be the following (though it's not very nice):

source_re = "^" + re.escape(source)
dest_quoted = dest.replace("\\", r"\\")
new_filename = re.sub(source_re, dest_quoted, fname, count=1)

This works on both Windows and Linux (so presumably on macOS as well). I'll submit a separate PR with this fix.

[juliangilbey]

Well, I found a separate issue in the file as I was editing it, so I've just submitted two separate PRs: #21695 and #21696. The tests for this PR should work once those two PRs are accepted and this PR is rebased against it. (The tests for #21696 will work because test_renamed_tree is currently broken and doesn't actually test the method correctly....)

[juliangilbey]

OK, I've now fixed test_renamed_tree so that it works on Windows by making all the filenames absolute (in a Windows sense) on that platform. This means that #21696 is probably unnecessary, but it is not harmful and it avoids an obscure corner case on non-Windows platforms.

[ccordoba12]

Looks good to me now, thanks @juliangilbey for your help with this!