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

tmpfs support #58

Open
mgorny opened this issue Feb 17, 2023 · 12 comments
Open

tmpfs support #58

mgorny opened this issue Feb 17, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@mgorny
Copy link

mgorny commented Feb 17, 2023

Description

When running the test suite, I'm getting inconsistent results, with 1-4 of the following tests failing:

FAILED tests/test_manager.py::test_get_path_oob_move_nested[False] - AssertionError: assert None == 'new_path/test_path'
FAILED tests/test_manager.py::test_get_path_oob_move_nested[True] - AssertionError: assert None == 'new_path/test_path'
FAILED tests/test_manager.py::test_get_path_oob_move_deeply_nested[True] - AssertionError: assert None == 'new_path/child/test_path'
FAILED tests/test_manager.py::test_get_path_oob_move_deeply_nested[False] - AssertionError: assert None == 'new_path/child/test_path'

Full output from one run (where two of them failed)

Full output from one run (where two of them failed)
========================================================= test session starts =========================================================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0
rootdir: /tmp/jupyter_server_fileid, configfile: pyproject.toml, testpaths: tests/
collected 53 items                                                                                                                    

tests/test_manager.py ............ss.................F..F..................                                                     [100%]

============================================================== FAILURES ===============================================================
_________________________________________________ test_get_path_oob_move_nested[True] _________________________________________________

fid_manager = <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab1eec90>, old_path = 'old_path', new_path = 'new_path'
stub_stat_crtime = True, fs_helpers = <jupyter_server_fileid.pytest_plugin.fs_helpers.<locals>.FsHelpers object at 0x7fddab1efad0>

    @pytest.mark.parametrize("stub_stat_crtime", [True, False], indirect=["stub_stat_crtime"])
    def test_get_path_oob_move_nested(fid_manager, old_path, new_path, stub_stat_crtime, fs_helpers):
        old_test_path = "test_path"
        new_test_path = os.path.join(new_path, "test_path")
        fs_helpers.touch(old_test_path)
        fid_manager.index(old_path)
        id = fid_manager.index(old_test_path)
    
        fs_helpers.move(old_path, new_path)
        fs_helpers.move(old_test_path, new_test_path)
    
>       assert fid_manager.get_path(id) == normalize_path(fid_manager, new_test_path)
E       AssertionError: assert None == 'new_path/test_path'
E        +  where None = <bound method LocalFileIdManager.get_path of <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab1eec90>>('e4f6ec8b-0e47-48f1-96a8-dae4de72a331')
E        +    where <bound method LocalFileIdManager.get_path of <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab1eec90>> = <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab1eec90>.get_path
E        +  and   'new_path/test_path' = normalize_path(<jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab1eec90>, 'new_path/test_path')

tests/test_manager.py:429: AssertionError
_____________________________________________ test_get_path_oob_move_deeply_nested[False] _____________________________________________

fid_manager = <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab0d0c50>, old_path = 'old_path', new_path = 'new_path'
old_path_child = 'old_path/child', new_path_child = 'new_path/child', stub_stat_crtime = False
fs_helpers = <jupyter_server_fileid.pytest_plugin.fs_helpers.<locals>.FsHelpers object at 0x7fddab0d1cd0>

    @pytest.mark.parametrize("stub_stat_crtime", [True, False], indirect=["stub_stat_crtime"])
    def test_get_path_oob_move_deeply_nested(
        fid_manager, old_path, new_path, old_path_child, new_path_child, stub_stat_crtime, fs_helpers
    ):
        old_test_path = "test_path"
        new_test_path = os.path.join(new_path_child, "test_path")
        fs_helpers.touch(old_test_path)
        fid_manager.index(old_path)
        fid_manager.index(old_path_child)
        id = fid_manager.index(old_test_path)
    
        fs_helpers.move(old_path, new_path)
        fs_helpers.move(old_test_path, new_test_path)
    
>       assert fid_manager.get_path(id) == normalize_path(fid_manager, new_test_path)
E       AssertionError: assert None == 'new_path/child/test_path'
E        +  where None = <bound method LocalFileIdManager.get_path of <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab0d0c50>>('dd1de351-0a20-419b-a42c-e58657a1d0a6')
E        +    where <bound method LocalFileIdManager.get_path of <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab0d0c50>> = <jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab0d0c50>.get_path
E        +  and   'new_path/child/test_path' = normalize_path(<jupyter_server_fileid.manager.LocalFileIdManager object at 0x7fddab0d0c50>, 'new_path/child/test_path')

tests/test_manager.py:448: AssertionError
========================================================== warnings summary ===========================================================
.venv/lib/python3.11/site-packages/jupyter_server/services/contents/filemanager.py:16
  /tmp/jupyter_server_fileid/.venv/lib/python3.11/site-packages/jupyter_server/services/contents/filemanager.py:16: DeprecationWarning: Jupyter is migrating its paths to use standard platformdirs
  given by the platformdirs library.  To remove this warning and
  see the appropriate new directories, set the environment variable
  `JUPYTER_PLATFORM_DIRS=1` and then run `jupyter --paths`.
  The use of platformdirs will be the default in `jupyter_core` v6
    from jupyter_core.paths import exists, is_file_hidden, is_hidden

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================== slowest 10 durations =========================================================

(10 durations < 0.005s hidden.  Use -vv to show these durations.)
======================================================= short test summary info =======================================================
SKIPPED [1] tests/test_manager.py:215: Requires crtime support.
SKIPPED [1] tests/test_manager.py:228: Requires crtime support.
FAILED tests/test_manager.py::test_get_path_oob_move_nested[True] - AssertionError: assert None == 'new_path/test_path'
FAILED tests/test_manager.py::test_get_path_oob_move_deeply_nested[False] - AssertionError: assert None == 'new_path/child/test_path'
========================================= 2 failed, 49 passed, 2 skipped, 1 warning in 0.34s ==========================================

Reproduce

  1. python -m venv .venv
  2. . venv/bin/activate
  3. pip install .[test]
  4. pytest

Expected behavior

Tests passing on every run ;-).

Context

  • Operating System and version: Gentoo Linux amd64
  • Browser and version: n/a
  • Jupyter Server version: 2.3.0
Troubleshoot Output
$PATH:
	/tmp/jupyter_server_fileid/.venv/bin
	/home/mgorny/perl5/bin
	/home/mgorny/node_modules/.bin/
	/home/mgorny/bin
	/usr/i686-pc-linux-gnu/gcc-bin/4.9.2
	/usr/local/sbin
	/usr/local/bin
	/usr/sbin
	/usr/bin
	/sbin
	/bin
	/opt/bin
	/usr/lib/llvm/17/bin
	/usr/lib/llvm/16/bin
	/usr/lib/llvm/15/bin
	/usr/lib/llvm/9/bin
	/etc/eselect/wine/bin
	/home/mgorny/.local/bin

sys.path:
/tmp/jupyter_server_fileid/.venv/bin
/usr/lib/python311.zip
/usr/lib/python3.11
/usr/lib/python3.11/lib-dynload
/tmp/jupyter_server_fileid/.venv/lib/python3.11/site-packages

sys.executable:
/tmp/jupyter_server_fileid/.venv/bin/python

sys.version:
3.11.2 (main, Feb 15 2023, 08:19:40) [GCC 12.2.1 20230121]

platform.platform():
Linux-6.1.12-gentoo-dist-x86_64-AMD_Ryzen_5_3600_6-Core_Processor-with-glibc2.36

which -a jupyter:
/tmp/jupyter_server_fileid/.venv/bin/jupyter
/usr/bin/jupyter

pip list:
Package Version
------------------------ ---------
anyio 3.6.2
argon2-cffi 21.3.0
argon2-cffi-bindings 21.2.0
arrow 1.2.3
asttokens 2.2.1
attrs 22.2.0
backcall 0.2.0
beautifulsoup4 4.11.2
bleach 6.0.0
certifi 2022.12.7
cffi 1.15.1
cfgv 3.3.1
charset-normalizer 3.0.1
comm 0.1.2
coverage 7.1.0
debugpy 1.6.6
decorator 5.1.1
defusedxml 0.7.1
distlib 0.3.6
executing 1.2.0
fastjsonschema 2.16.2
filelock 3.9.0
fqdn 1.5.1
identify 2.5.18
idna 3.4
iniconfig 2.0.0
ipykernel 6.21.2
ipython 8.10.0
isoduration 20.11.0
jedi 0.18.2
Jinja2 3.1.2
jsonpointer 2.3
jsonschema 4.17.3
jupyter_client 8.0.3
jupyter_core 5.2.0
jupyter-events 0.6.3
jupyter_server 2.3.0
jupyter_server_fileid 0.7.0
jupyter_server_terminals 0.4.4
jupyterlab-pygments 0.2.2
MarkupSafe 2.1.2
matplotlib-inline 0.1.6
mistune 2.0.5
nbclient 0.7.2
nbconvert 7.2.9
nbformat 5.7.3
nest-asyncio 1.5.6
nodeenv 1.7.0
packaging 23.0
pandocfilters 1.5.0
parso 0.8.3
pexpect 4.8.0
pickleshare 0.7.5
pip 23.0
platformdirs 3.0.0
pluggy 1.0.0
pre-commit 3.0.4
prometheus-client 0.16.0
prompt-toolkit 3.0.36
psutil 5.9.4
ptyprocess 0.7.0
pure-eval 0.2.2
pycparser 2.21
Pygments 2.14.0
pyrsistent 0.19.3
pytest 7.2.1
pytest-console-scripts 1.3.1
pytest-cov 4.0.0
pytest-jupyter 0.6.2
pytest-timeout 2.1.0
python-dateutil 2.8.2
python-json-logger 2.0.6
PyYAML 6.0
pyzmq 25.0.0
requests 2.28.2
rfc3339-validator 0.1.4
rfc3986-validator 0.1.1
Send2Trash 1.8.0
setuptools 67.3.2
six 1.16.0
sniffio 1.3.0
soupsieve 2.4
stack-data 0.6.2
terminado 0.17.1
tinycss2 1.2.1
tornado 6.2
traitlets 5.9.0
uri-template 1.2.0
urllib3 1.26.14
virtualenv 20.19.0
wcwidth 0.2.6
webcolors 1.12
webencodings 0.5.1
websocket-client 1.5.1

@mgorny mgorny added the bug Something isn't working label Feb 17, 2023
@welcome
Copy link

welcome bot commented Feb 17, 2023

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

What filesystem are you using? That information is very relevant for this project, but I neglected to add it to the issue template. 😅

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

LocalFileIdManager will not work on all platforms, though we make a best effort attempt at doing so. You can see by searching for @pytest.mark.skipif in the test file that we need to skip certain tests on certain platforms.

@mgorny
Copy link
Author

mgorny commented Feb 17, 2023

What filesystem are you using? That information is very relevant for this project, but I neglected to add it to the issue template. sweat_smile

I'm running tests on tmpfs.

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

Yup, LocalFileIdManager so far has only be tested on the "mainstream" filesystems, namely: ext4, NTFS, HFS, and APFS. Depending on whether tmpfs support for LocalFileIdManager is possible, the solution here is to either skip the tests on tmpfs platforms or to implement tmpfs support.

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

You will still be able to use the jupyter_server_fileid server extension; we default to using ArbitraryFileIdManager, which in principle should not rely on platform-specific details like OS or filesystem.

@dlqqq dlqqq changed the title Inconsistent tests/test_manager.py::test_get_path_oob_move* test failures tmpfs support Feb 17, 2023
@mgorny
Copy link
Author

mgorny commented Feb 17, 2023

So I should basically be skipping these tests in the Gentoo ebuild for jupyter-server-fileid? I don't really want to guess whether user's filesystem is supported or not.

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

For now, yes.

I don't really want to guess whether user's filesystem is supported or not.

Unfortunately, we have not yet run a comprehensive test of LocalFileIdManager on every plausible filesystem. I hope you appreciate that this task would involve a considerable amount of effort. 🙏 We had been planning on doing so, but this has been assigned a lower priority after we made the decision to default to ArbitraryFileIdManager to ensure compatibility on all platforms.

As a temporary workaround, perhaps I could somehow "separate" the Local and Arbitrary FIM tests for you? That way, on build platforms, you can just run the ArbitraryFIM tests exclusively instead, which all should pass.

@mgorny
Copy link
Author

mgorny commented Feb 17, 2023

Unfortunately, we have not yet run a comprehensive test of LocalFileIdManager on every plausible filesystem.

To be honest, I don't think that would ever be possible. If you're relying on anything non-portable, then sooner or later it'll break in unexpected ways (kernel changes, people using union filesystems…).

As a temporary workaround, perhaps I could somehow "separate" the Local and Arbitrary FIM tests for you? That way, on build platforms, you can just run the ArbitraryFIM tests exclusively instead, which all should pass.

I'm sorry but I don't really know the package, we've ended up packaging it because it's needed by jupyter-server-ydoc. Do I understand correctly that the test suite right now is effectively run against "Local" FIM? If yes, then indeed I think it'd be beneficial to run against both FIMs. Perhaps using a parametrized fixture — that should be sufficient to make it easy for us to --deselect problematic tests.

@dlqqq
Copy link
Collaborator

dlqqq commented Feb 17, 2023

To be honest, I don't think that would ever be possible. If you're relying on anything non-portable, then sooner or later it'll break in unexpected ways (kernel changes, people using union filesystems…).

Yeah, I know. 😔 Which is why LocalFileIdManager will likely remain opt-in only.

I'm sorry but I don't really know the package, we've ended up packaging it because it's needed by jupyter-server-ydoc. Do I understand correctly that the test suite right now is effectively run against "Local" FIM?

There are tests for both FIMs in the test suite.

Perhaps using a parametrized fixture — that should be sufficient to make it easy for us to --deselect problematic tests.

Is there no way to tell pytest to skip tests by name? The test names should be fairly stable, so your build process could just skip those 2 failing tests for now until we can allocate effort to work on this.

@mgorny
Copy link
Author

mgorny commented Apr 10, 2023

I'm sorry, I've missed your reply.

Do I understand correctly that basically tests/test_manager.py::test_get_path_oob_move_nested and tests/test_manager.py::test_get_path_oob_move_deeply_nested are always run against local FIM? In that case, the current --deselect is sufficient for us.

What I was referring to is making sure that we skip these tests for "local FIM" without skipping the "arbitrary FIM" tests.

@dlqqq
Copy link
Collaborator

dlqqq commented Apr 11, 2023

Do I understand correctly that basically tests/test_manager.py::test_get_path_oob_move_nested and tests/test_manager.py::test_get_path_oob_move_deeply_nested are always run against local FIM? In that case, the current --deselect is sufficient for us.

@mgorny Yes. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants