René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Tests create tmp dir in GitPython dir and fail on second run","articleBody":"Since 08a819c (#1799), running the test suite a second time locally causes a test to fail the second time that passed the first time. This test is `TestTree.test_tree_modifier_ordering`, and the problem is that:\r\n\r\n- It creates its `tmp` directory directly in the current working directory of the test runner, which in practice is the top of the GitPython source tree, rather than in `/tmp` or whatever other location is configured to hold temporary files:\r\n\r\n  https://github.com/gitpython-developers/GitPython/blob/7ba3fd2a4bd7a384d105ab0a1e4909940a92cda2/test/test_tree.py#L45-L48\r\n\r\n- While it *typically* manages to change directory back (though this is not guarded by `try`-`finally` or equivalent, it never attempts to delete this directory:\r\n\r\n  https://github.com/gitpython-developers/GitPython/blob/7ba3fd2a4bd7a384d105ab0a1e4909940a92cda2/test/test_tree.py#L95-L96\r\n\r\nThis does not break CI, because every CI check uses a new clone of the repository. Manually deleting the directory works as a workaround. The bug can be observed in local testing as follows:\r\n\r\n```text\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ git status\r\nOn branch main\r\nYour branch is up to date with 'origin/main'.\r\n\r\nnothing to commit, working tree clean\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ pytest --no-cov -vvk test_tree_modifier_ordering\r\nTest session starts (platform: linux, Python 3.12.1, pytest 8.0.0, pytest-sugar 1.0.0)\r\ncachedir: .pytest_cache\r\nrootdir: /home/ek/repos-wsl/GitPython\r\nconfigfile: pyproject.toml\r\ntestpaths: test\r\nplugins: mock-3.12.0, sugar-1.0.0, cov-4.1.0, instafail-0.5.0\r\ncollected 601 items / 600 deselected / 1 selected\r\n\r\n test/test_tree.py::TestTree.test_tree_modifier_ordering ✓                                               100% ██████████\r\n\r\nResults (0.46s):\r\n       1 passed\r\n     600 deselected\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $%=)$ git status\r\nOn branch main\r\nYour branch is up to date with 'origin/main'.\r\n\r\nUntracked files:\r\n  (use \"git add \u003cfile\u003e...\" to include in what will be committed)\r\n        tmp/\r\n\r\nnothing added to commit but untracked files present (use \"git add\" to track)\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $%=)$ pytest --no-cov -vvk test_tree_modifier_ordering\r\nTest session starts (platform: linux, Python 3.12.1, pytest 8.0.0, pytest-sugar 1.0.0)\r\ncachedir: .pytest_cache\r\nrootdir: /home/ek/repos-wsl/GitPython\r\nconfigfile: pyproject.toml\r\ntestpaths: test\r\nplugins: mock-3.12.0, sugar-1.0.0, cov-4.1.0, instafail-0.5.0\r\ncollected 601 items / 600 deselected / 1 selected\r\n\r\n\r\n――――――――――――――――――――――――――――――――――――――――― TestTree.test_tree_modifier_ordering ―――――――――――――――――――――――――――――――――――――――――\r\n\r\nself = \u003ctest.test_tree.TestTree testMethod=test_tree_modifier_ordering\u003e\r\n\r\n    def test_tree_modifier_ordering(self):\r\n        def setup_git_repository_and_get_ordered_files():\r\n            os.mkdir(\"tmp\")\r\n            os.chdir(\"tmp\")\r\n            subprocess.run([\"git\", \"init\", \"-q\"], check=True)\r\n            os.mkdir(\"file\")\r\n            for filename in [\r\n                \"bin\",\r\n                \"bin.d\",\r\n                \"file.to\",\r\n                \"file.toml\",\r\n                \"file.toml.bin\",\r\n                \"file0\",\r\n                \"file/a\",\r\n            ]:\r\n                open(filename, \"a\").close()\r\n\r\n            subprocess.run([\"git\", \"add\", \".\"], check=True)\r\n            subprocess.run([\"git\", \"commit\", \"-m\", \"c1\"], check=True)\r\n            tree_hash = subprocess.check_output([\"git\", \"rev-parse\", \"HEAD^{tree}\"]).decode().strip()\r\n            cat_file_output = subprocess.check_output([\"git\", \"cat-file\", \"-p\", tree_hash]).decode()\r\n            return [line.split()[-1] for line in cat_file_output.split(\"\\n\") if line]\r\n\r\n        hexsha = \"6c1faef799095f3990e9970bc2cb10aa0221cf9c\"\r\n        roottree = self.rorepo.tree(hexsha)\r\n        blob_mode = Tree.blob_id \u003c\u003c 12\r\n        tree_mode = Tree.tree_id \u003c\u003c 12\r\n\r\n        files_in_desired_order = [\r\n            (blob_mode, \"bin\"),\r\n            (blob_mode, \"bin.d\"),\r\n            (blob_mode, \"file.to\"),\r\n            (blob_mode, \"file.toml\"),\r\n            (blob_mode, \"file.toml.bin\"),\r\n            (blob_mode, \"file0\"),\r\n            (tree_mode, \"file\"),\r\n        ]\r\n        mod = roottree.cache\r\n        for file_mode, file_name in files_in_desired_order:\r\n            mod.add(hexsha, file_mode, file_name)\r\n        # end for each file\r\n\r\n        def file_names_in_order():\r\n            return [t[1] for t in files_in_desired_order]\r\n\r\n        def names_in_mod_cache():\r\n            a = [t[2] for t in mod._cache]\r\n            here = file_names_in_order()\r\n            return [e for e in a if e in here]\r\n\r\n\u003e       git_file_names_in_order = setup_git_repository_and_get_ordered_files()\r\n\r\ntest/test_tree.py:95:\r\n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _\r\n\r\n    def setup_git_repository_and_get_ordered_files():\r\n\u003e       os.mkdir(\"tmp\")\r\nE       FileExistsError: [Errno 17] File exists: 'tmp'\r\n\r\ntest/test_tree.py:47: FileExistsError\r\n\r\n test/test_tree.py::TestTree.test_tree_modifier_ordering ⨯                                               100% ██████████\r\n=============================================== short test summary info ================================================\r\nFAILED test/test_tree.py::TestTree::test_tree_modifier_ordering - FileExistsError: [Errno 17] File exists: 'tmp'\r\n\r\nResults (0.36s):\r\n       1 failed\r\n         - test/test_tree.py:45 TestTree.test_tree_modifier_ordering\r\n     600 deselected\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $%=)$ rm -rf tmp\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ pytest --no-cov -vvk test_tree_modifier_ordering\r\nTest session starts (platform: linux, Python 3.12.1, pytest 8.0.0, pytest-sugar 1.0.0)\r\ncachedir: .pytest_cache\r\nrootdir: /home/ek/repos-wsl/GitPython\r\nconfigfile: pyproject.toml\r\ntestpaths: test\r\nplugins: mock-3.12.0, sugar-1.0.0, cov-4.1.0, instafail-0.5.0\r\ncollected 601 items / 600 deselected / 1 selected\r\n\r\n test/test_tree.py::TestTree.test_tree_modifier_ordering ✓                                               100% ██████████\r\n\r\nResults (0.35s):\r\n       1 passed\r\n     600 deselected\r\n```\r\n\r\nThis is a test-only bug, and I believe the test does correctly verify the fix in 365d44f for #369. This test bug only causes the test to wrongly fail (when run a second time with no intervening deletion), not to wrongly pass. It can be fixed pretty straightforwardly by using context managers and/or decorators from the standard library and/or GitPython's test helpers to ensure that cleanup, including deletion of the directory, is deterministically performed.\r\n\r\nAlthough fixing this test bug is fairly straightforward, the simple approach of using `TemporaryDirectory` to manage the directory is not quite sufficient, because in Python 3.7 on Windows it doesn't succeed at deleting files whose permissions need to be changed for them to be deleted, which includes some read-only files in a `.git` directory. Since GitPython already has `git.util.rmtree` to take care of this, which can be used either directly or indirectly through `@with_rw_directory`, the question of how best to do it mostly comes down to what is to be considered clearest. **I've proposed a fix in #1825.**","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2024-02-15T17:23:08.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/1824/GitPython/issues/1824"}