GeezerVison is showing github.com — 3.1.40: `TestRepo.test_new_should_raise_on_invalid_repo_location` fails · Issue #1744 · gitpython-developers/GitPython
3.1.40: TestRepo.test_new_should_raise_on_invalid_repo_location fails #1744DescriptionI'm running tests for GitPython 3.1.40 and the following test fails:
___________ TestRepo.test_new_should_raise_on_invalid_repo_location ____________
self = <test.test_repo.TestRepo testMethod=test_new_should_raise_on_invalid_repo_location>
def test_new_should_raise_on_invalid_repo_location(self):
> self.assertRaises(InvalidGitRepositoryError, Repo, tempfile.gettempdir())
E AssertionError: InvalidGitRepositoryError not raised by Repo
test/test_repo.py:81: AssertionError
ActivityByron commented on Nov 27, 2023 Did you follow the instructions? Running tests isn't like one would expect, unfortunately.
My preference would be to open one issue related to packaging on OpenIndiana (as mentioned in this comment) where we can figure out what to do as my feeling is that GitPython is treated like any other package even though it unfortunately is a bit special. mtelka commented on Nov 27, 2023
Did you follow the instructions? Running tests isn't like one would expect, unfortunately.
Yes, I do this.
My preference would be to open one issue related to packaging on OpenIndiana (as mentioned in this comment) where we can figure out what to do as my feeling is that GitPython is treated like any other package even though it unfortunately is a bit special.
Hmm, I do not see GitPython as very special when compared to other Python projects I already packaged. I found only few issues, but nothing critical:
- The
test-requirements.txt file contains packages that are not needed to run tests: pytest-instafail and pytest-sugar (maybe there are more, likely pre-commit for example, but I do not care about them). This is harmless and nothing special.
- One test is failing (this issue). Again, nothing special. I see many Python projects with few tests failing.
- sdist contains no tests. Nothing special.
- Special setup is needed before runing tests. Yes, something like this is rare, but easily doable (see above).
- I need to remove
/tmp/repos before I run tests. Otherwise testing fails. I think there are two tests failing. This is annoying, but I can live with that. I think the problem is that testing leaves non-empty /tmp/repos there so when I try to run tests again (the 2nd time) they finds some unexpected content in /tmp/repos.
- I found that when I set environment variable
TMPDIR to non-default value then some tests start to fail.
- I need to pass
-- --color=no to tox to get test results without colors. Minor issue. I see this often.
So to sum, we do not need special issue here for OpenIndiana packaging, since I already packaged GitPython successfully.
Thank you. mtelka commented on Nov 27, 2023 ... and yes, I know about #1324. Byron commented on Nov 27, 2023 Thanks for elaborating, it indeed looks like this is the only true issue left when packaging for OpenIndiana.
The failing test seems to find a parent repository when looking at the temp directory (even without searching upwards), which is unusual. Maybe the test could be adjusted to create a temporary, known-to-be-empty, directory? mtelka commented on Nov 27, 2023
The failing test seems to find a parent repository when looking at the temp directory (even without searching upwards), which is unusual. Maybe the test could be adjusted to create a temporary, known-to-be-empty, directory?
The testing is done in the oi-userland git tree. In the tree there is GitPython git repo cloned and GIT_PYTHON_TEST_GIT_REPO_BASE is pointed to it. Is the problem that when testing goes .. out of GIT_PYTHON_TEST_GIT_REPO_BASE (aka the GitPython git repo) then there is another unrelated git repo found? Byron commented on Nov 28, 2023 It really depends on whether or not tempfile.gettempdir() is a valid git repository in the execution context. By default, it won't search upwards either, which leaves as only conclusion that the tempdir is a valid git repository there.
Adjusting the test to create its own empty tempdir first should do the trick though. mtelka commented on Nov 28, 2023
It really depends on whether or not tempfile.gettempdir() is a valid git repository in the execution context. By default, it won't search upwards either, which leaves as only conclusion that the tempdir is a valid git repository there.
I believe in my case tempfile.gettempdir() returns /tmp and /tmp is not a valid git repo (neither / is).
Adjusting the test to create its own empty tempdir first should do the trick though.
Is this something I should do on my side (e.g. by pointing TMPDIR to somewhere), or something that ought be done in GitPython's tests? Byron commented on Nov 28, 2023 I checked again, and if a directory is given, it will use it. Otherwise it falls back to the GIT_DIR environment variable. Thus it shouldn't be an issue to have it set.
Is this something I should do on my side (e.g. by pointing TMPDIR to somewhere), or something that ought be done in GitPython's tests?
Maybe it's easiest to point TMPDIR to a known-to-be-empty directory. From all I can tell by looking at the code, it won't try to break out of the given directory. EliahKagan commented on Dec 8, 2023 Although I think the changes in #1759 would be worthwhile even if they are insufficient to fix this, I believe those changes should be sufficient. If it turns out they are not, then I would likely be interested in contributing further fixes to allow tests to pass when packaging for OpenIndiana. Reacted by Sebastian Thiel mtelka commented on Dec 11, 2023 I tested 3.1.40 with all three changesets from #1759 and I'm glad to report that the test_new_should_raise_on_invalid_repo_location pass now. The newly added test_new_should_raise_on_invalid_repo_location_within_repo test pass too.
Thank you @EliahKagan for the fix!
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|