Consistent error messages

[nicolashainaux]

Hello, I am not sure if this is the right place to tell this, but I noticed the error messages raised by different mocked functions in pyfakefs raise different error messages, like in these examples below:

def test_permission_error1(fs):
    fs.create_dir('/home1/')
    Path('/home1/').chmod(0o555)
    fs.create_dir('/drive1/')
    fs.create_file('/drive1/new.txt')
    with pytest.raises(PermissionError) as excinfo:
        shutil.copy2(Path('/drive1/new.txt'), Path('/home1/'))
    assert str(excinfo.value) == "[Errno 13] Permission Denied: '/home1'"


def test_permission_error2(fs):
    fs.create_dir('/side1/')
    fs.create_dir('/side1/dir1/')
    fs.create_dir('/side1/dir1/dir2/')
    Path('/side1/dir1/').chmod(0o555)

    with pytest.raises(PermissionError) as excinfo:
        shutil.move(Path('/side1/dir1/dir2'),
                    Path('/side1/dir1/dir3'))
    assert str(excinfo.value) \
        == "[Errno 13] Permission Denied: '/side1/dir1/dir3'"

The first test passes successfully, but the second test fails this way:

$ pytest -vv tests/06_commands_test.py::test_permission_error1 tests/06_commands_test.py::test_permission_error2
================================================================================== test session starts ==================================================================================
platform linux -- Python 3.9.13, pytest-7.4.4, pluggy-1.5.0 -- /home/nico/.local/dev/zano/.venv/py39/bin/python
cachedir: .pytest_cache
rootdir: /home/nico/.local/dev/zano
plugins: pyfakefs-5.8.0, mock-3.14.0
collected 2 items                                                                                                                                                                       

tests/06_commands_test.py::test_permission_error1 PASSED                                                                                                                          [ 50%]
tests/06_commands_test.py::test_permission_error2 FAILED                                                                                                                          [100%]

======================================================================================= FAILURES ========================================================================================
________________________________________________________________________________ test_permission_error2 _________________________________________________________________________________

fs = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7f08275867f0>

    def test_permission_error2(fs):
        fs.create_dir('/side1/')
        fs.create_dir('/side1/dir1/')
        fs.create_dir('/side1/dir1/dir2/')
        Path('/side1/dir1/').chmod(0o555)
    
        with pytest.raises(PermissionError) as excinfo:
            shutil.move(Path('/side1/dir1/dir2'),
                        Path('/side1/dir1/dir3'))
>       assert str(excinfo.value) \
            == "[Errno 13] Permission Denied: '/side1/dir1/dir3'"
E       assert "[Errno 13] Permission denied in the fake filesystem: '/side1/dir1/dir3'" == "[Errno 13] Permission Denied: '/side1/dir1/dir3'"
E         - [Errno 13] Permission Denied: '/side1/dir1/dir3'
E         ?                       ^
E         + [Errno 13] Permission denied in the fake filesystem: '/side1/dir1/dir3'
E         ?                       ^     +++++++++++++++++++++++

/home/nico/.local/dev/zano/tests/06_commands_test.py:2853: AssertionError
================================================================================ short test summary info ================================================================================
FAILED tests/06_commands_test.py::test_permission_error2 - assert "[Errno 13] Permission denied in the fake filesystem: '/side1/dir1/dir3'" == "[Errno 13] Permission Denied: '/side1/dir1/dir3'"
============================================================================== 1 failed, 1 passed in 0.48s ==============================================================================

This is not really an issue, as I can check "[Errno 13] Permission" is in the error message, as well as the directory name.

I also suppose that replacing [Errno 13] Permission denied in the fake filesystem with [Errno 13] Permission Denied would be a problem, as it might break already written tests relying on the original message.

Moreover, in some places, especially when one handles both fake and real files in the same test, it proves useful to be able to make the difference between error messages from the fake and from the real filesystem (I mean, the real python functions).

Still, I wonder if there is a special reason why these two (from copy2() and move()) messages differ.

[mrbean-bremen]

Actually this is a good point. I think the intention at the time was to make clear that the message comes from the fake fs, to distinguish them from accidental access to the real fs (but you will see this anyway in the callstack). Also, having the addition at the end makes it possible to check the error messages using startswith.

But - as you have noticed, this is not consistent, because there are several places where the exception is not raised using the method to raise an OS exception, as is the case here.
Note also that in this case the capitalization differs - this is because in the first (passing) case the message has been hard-coded, while in the second case in is taken using os.strerror(), which is the safer approach. Specifically, it is composed using:

    message = os.strerror(err_no) + " in the fake filesystem"

Obviously, at the time I didn't replace all the occurences of manually adding the error message, there are still a few instances where an OSError is raised with a hardcoded message (I just counted 13 of these - as opposed to more than 100 uses of the generic method).

So, I think what I will do is replacing these instances with the generic call, and probably get rid of the " in the fake filesystem" part. I very much doubt that anybody is relying on this, though the change has to be documented, of course.

Let me know if this would be ok for you.

[nicolashainaux]

Sorry for the delay, yes this is OK!

[mrbean-bremen]

Ok, is now in main branch.