Closed
Labels
Description
Hi! I maintain this package for Arch Linux. Many new failing test with 3.1.38:
=========================== short test summary info ============================
FAILED ../../../../dev/test/test_docs.py::Tutorials::test_references_and_objects
FAILED ../../../../dev/test/test_docs.py::Tutorials::test_submodules - IndexE...
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_clone_from_with_path_contains_unicode
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_submodule_update - g...
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_submodules - Asserti...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_add_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_add_no_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_base_rw - ...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_branch_renames
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_depth - gi...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_git_submodule_compatibility
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_remove_norefs
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_rename - g...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_root_module
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_update_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_update_no_clone_multi_options_argument
==== 16 failed, 468 passed, 42 skipped, 2 deselected, 5 warnings in 47.69s =====
python-gitpython-3.1.38-1-x86_64-check.log
As I neither use this package, nor really know how to fix these tests (or have the time to do so), I'll disable them.
Activity
Byron commentedon Oct 18, 2023
Thanks for reporting.
Is it possible to add Arch Linux to CI here? If you happen to know existing GitHub CI setups, I'd appreciate a pointer so maybe one day this platform can also be tested here.
Maybe this is also related to recent changes though, so CC @EliahKagan .
Don't pre-clone submodules on CI, at least for now
Have init script clone submodules unconditionally
EliahKagan commentedon Oct 18, 2023
I'm pretty sure I inadvertently introduced this bug in #1693, when I had
init-tests-after-clone.shrefrain from cloning submodules when it detected it was running on CI, giving the current code:GitPython/init-tests-after-clone.sh
Lines 50 to 53 in 6f765a2
I reasoned, incorrectly, that this was safe, due to the CI test workflows in this repository (and thus forks, reuploads, etc.) taking care of cloning submodules (this shows
pythonpackage.yml, butcygwin-test.ymlhas the same thing):GitPython/.github/workflows/pythonpackage.yml
Lines 27 to 30 in 6f765a2
The problem is that checking for CI--even conservatively, as done here--is not the same as checking for CI workflows originating in this repository:
GitPython/init-tests-after-clone.sh
Lines 7 to 10 in 6f765a2
Confusing the two does not cause data loss--any CI will want to skip the interactive prompt--but it does cause brittleness. GitPython's tests can be run on CI through workflows unrelated to the ones we have here, and projects distributing downstream packages of GitPython, including operating systems such as Arch Linux, are examples of where it makes sense to do that.
init-tests-after-clone.shis intended to be sufficient, after an ordinary (non-shallow) clone, to get a cloned repository to a state where the tests are ready to be run. I think this is both an intended and reasonable assumption from:GitPython/README.md
Lines 120 to 122 in 6f765a2
The need to clone submodules is intentionally not mentioned in
README.md, because it is intended that the init script take care of that whenever it is necessary. Therefore, this really is a regression in GitPython's tests--or more precisely in code that supports them--which I introduced in #1693 due to a mistake about what we are in practice checking for when we check for CI.The fix should be simple: the init script should unconditionally clone submodules. More granular solutions that still avoid cloning submodules on some CI configurations or if they appear already present are possible, but if that is to be done then I suggest deferring it, so the simpler fix can be applied sooner. I've proposed that fix in #1715, tested it (though not with Arch Linux), and included a more comprehensive analysis in the description.
Byron commentedon Oct 18, 2023
This issue should now be fixed with the latest release. Please let us know if that's not the case here.
Add a check that required submodules are initialized
Add a check that required submodules are initialized
Add a check that required submodules are initialized
Add a check that required submodules are initialized
Add a test that required submodules are initialized
Add a test that required submodules are initialized
Add a test that required submodules are initialized
10 remaining items
Add a test that required submodules are initialized
Add submodules: recursive temporarily, for testing
Add a test that required submodules are initialized
Add submodules: recursive temporarily, for testing
Add submodules: recursive temporarily, for testing
Add a test that required submodules are initialized
Add `submodules: recursive` temporarily, for testing
Add a test that required submodules are initialized
Add `submodules: recursive` temporarily, for testing
Add a test that required submodules are initialized
Add `submodules: recursive` temporarily, for testing
Add `submodules: recursive` temporarily, for testing
Add `submodules: recursive` temporarily, for testing
Add a test that required submodules are initialized
Add `submodules: recursive` temporarily, for testing