Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.12.0: test_bad_lock_file[current_directory-UnixFileLock] fails #233

Closed
mtelka opened this issue Apr 21, 2023 · 6 comments
Closed

3.12.0: test_bad_lock_file[current_directory-UnixFileLock] fails #233

mtelka opened this issue Apr 21, 2023 · 6 comments

Comments

@mtelka
Copy link

mtelka commented Apr 21, 2023

During upgrade of the filelock package for OpenIndiana from version 3.11.0 to version 3.12.0 I noticed that one test fails (with version 3.11.0 all tests passed):

______________ test_bad_lock_file[current_directory-UnixFileLock] ______________

lock_type = <class 'filelock._unix.UnixFileLock'>
expected_error = <class 'IsADirectoryError'>, match = 'Is a directory'
bad_lock_file = '.'

    @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
    @pytest.mark.parametrize(
        ("expected_error", "match", "bad_lock_file"),
        [   
            pytest.param(FileNotFoundError, "No such file or directory:", "a/b", id="non_existent_directory"),
            pytest.param(FileNotFoundError, "No such file or directory:", "", id="blank_filename"),
            pytest.param(ValueError, "embedded null (byte|character)", "\0", id="null_byte"),
            pytest.param(
                PermissionError if sys.platform == "win32" else IsADirectoryError,
                "Permission denied:" if sys.platform == "win32" else "Is a directory",
                ".",
                id="current_directory",
            ),
        ]
        + [pytest.param(OSError, "Invalid argument", i, id=f"invalid_{i}", marks=WindowsOnly) for i in '<>:"|?*\a']
        + [pytest.param(PermissionError, "Permission denied:", i, id=f"permission_{i}", marks=WindowsOnly) for i in "/\\"],
    )
    @pytest.mark.timeout(5)  # timeout in case of infinite loop
    def test_bad_lock_file(
        lock_type: type[BaseFileLock],
        expected_error: type[Exception],
        match: str,
        bad_lock_file: str,
    ) -> None:
        lock = lock_type(bad_lock_file)

        with pytest.raises(expected_error, match=match):
>           lock.acquire()

tests/test_filelock.py:145:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../prototype/i386/usr/lib/python3.9/vendor-packages/filelock/_api.py:217: in acquire
    self._acquire()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <filelock._unix.UnixFileLock object at 0x7fffa83972e0>

    def _acquire(self) -> None:
        open_flags = os.O_RDWR | os.O_CREAT | os.O_TRUNC
>       fd = os.open(self.lock_file, open_flags, self._context.mode)
E       OSError: [Errno 22] Invalid argument: '.'

../prototype/i386/usr/lib/python3.9/vendor-packages/filelock/_unix.py:36: OSError
@TheMatt2
Copy link
Contributor

TheMatt2 commented May 21, 2023

I see the current fix is to omit this test which will work, since this is an edgecase of the test, not of filelock.

I'd like a little more information though so we can see about a permanent fix. Sorry I am not familiar with OpenIndiana / SunOS.

What is sys.platform for this OS?

For this OS, I assume the expected error for trying to open "." as if it were a file is OSError: [Errno 22] Invalid argument: '.'?

os.open('.', os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o644)

@mtelka
Copy link
Author

mtelka commented May 21, 2023

$ python -c 'import sys; print(sys.platform)'
sunos5
$ python -c "import os; os.open('.', os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o644)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
OSError: [Errno 22] Invalid argument: '.'
$

@TheMatt2
Copy link
Contributor

TheMatt2 commented May 21, 2023

This is interesting. Based on the Oracle page, I would have thought the error message would be EISDIR. If you can explain this, I'd love to know.

@TheMatt2
Copy link
Contributor

Can you try another thing for me? Want to make sure I understand how this is behaving.

os.open(os.path.curdir, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
open(os.path.curdir)
print(os.path.curdir)

@TheMatt2
Copy link
Contributor

Considering the reason for this test case was because of a Windows specific error (#231), I think it may be reasonable to change the check so "if non-windows, make sure the result isn't EPERM".

@gaborbernat
Copy link
Member

This should be done now, the CI passes at least with 3.12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants