3.1.40: TestRepo.test_new_should_raise_on_invalid_repo_location fails

[mtelka]

I'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

[Byron]

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]

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:

1. 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.
2. One test is failing (this issue). Again, nothing special. I see many Python projects with few tests failing.
3. sdist contains no tests. Nothing special.
4. Special setup is needed before runing tests. Yes, something like this is rare, but easily doable (see above).
5. 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.
6. I found that when I set environment variable TMPDIR to non-default value then some tests start to fail.
7. 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]

... and yes, I know about #1324.

[Byron]

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]

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]

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]

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]

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]

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.

[mtelka]

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!