From 45773c27fa40fddf72b40970836b3649094cd994 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 7 Sep 2023 08:16:29 -0400 Subject: [PATCH 01/32] Instrument workflows to investigate skipped tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes jobs from both test workflows give more information relevant to examining which tests are skipped (and if any tests xfail, those too) in what environments: - Values of os.name and git.util.is_win. - The name of each test that runs, with its status. The latter doesn't increase the output length as much as might be expected, because due to the way the output is handled, the pytest-sugar pretty output format without -v looked like: test/test_actor.py ✓ 0% test/test_actor.py ✓✓ 0% ▏ test/test_actor.py ✓✓✓ 1% ▏ test/test_actor.py ✓✓✓✓ 1% ▏ When instead it was intended to fit on a single line. Still, the current output with -v has extra newlines, increasing length and worsening readability, so it should be improved on if possible. --- .github/workflows/cygwin-test.yml | 6 +++++- .github/workflows/pythonpackage.yml | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 962791ae7..3f93f6830 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -58,7 +58,11 @@ jobs: run: | /usr/bin/python -m pip install ".[test]" + - name: Check 'is_win' + run: | + /usr/bin/python -c 'import os, git; print(f"os.name={os.name}, is_win={git.compat.is_win}")' + - name: Test with pytest run: | set +x - /usr/bin/python -m pytest + /usr/bin/python -m pytest -v diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index a5467ef94..c57f31cdc 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -73,9 +73,13 @@ jobs: # so we have to ignore errors until that changes. continue-on-error: true + - name: Check 'is_win' + run: | + python -c 'import os, git; print(f"os.name={os.name}, is_win={git.compat.is_win}")' + - name: Test with pytest run: | - pytest + pytest -v continue-on-error: false - name: Documentation From 6fbe5118514618d34ea8912e99fbde3fd6d7a557 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Sep 2023 16:09:01 -0400 Subject: [PATCH 02/32] Show version and platform info in one place Instead of splitting it in into two places where at least one of the places is highly likely to be missed, this puts it together just before the first steps that makes nontrivial use of the installed packages. Grouping it together, it can't really be shown earlier, because one of the pieces of information is obtained using the git module (to examine that behavior of the code). This also presents the information more clearly. "set -x" makes this easy, so the commands are rewritten to take advantage of it. --- .github/workflows/cygwin-test.yml | 13 ++++++------- .github/workflows/pythonpackage.yml | 17 ++++++++--------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 3f93f6830..1563afc95 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -29,11 +29,6 @@ jobs: with: packages: python39 python39-pip python39-virtualenv git - - name: Show python and git versions - run: | - /usr/bin/python --version - /usr/bin/git version - - name: Tell git to trust this repo run: | /usr/bin/git config --global --add safe.directory "$(pwd)" @@ -58,9 +53,13 @@ jobs: run: | /usr/bin/python -m pip install ".[test]" - - name: Check 'is_win' + - name: Show version and platform information run: | - /usr/bin/python -c 'import os, git; print(f"os.name={os.name}, is_win={git.compat.is_win}")' + /usr/bin/git version + /usr/bin/python --version + /usr/bin/python -c 'import sys; print(sys.platform)' + /usr/bin/python -c 'import os; print(os.name)' + /usr/bin/python -c 'import git; print(git.compat.is_win)' - name: Test with pytest run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index c57f31cdc..696057ae2 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -36,11 +36,6 @@ jobs: python-version: ${{ matrix.python-version }} allow-prereleases: ${{ matrix.experimental }} - - name: Show python and git versions - run: | - python --version - git version - - name: Prepare this repo for tests run: | TRAVIS=yes ./init-tests-after-clone.sh @@ -66,6 +61,14 @@ jobs: run: | pip install ".[test]" + - name: Show version and platform information + run: | + git version + python --version + python -c 'import sys; print(sys.platform)' + python -c 'import os; print(os.name)' + python -c 'import git; print(git.compat.is_win)' + - name: Check types with mypy run: | mypy -p git @@ -73,10 +76,6 @@ jobs: # so we have to ignore errors until that changes. continue-on-error: true - - name: Check 'is_win' - run: | - python -c 'import os, git; print(f"os.name={os.name}, is_win={git.compat.is_win}")' - - name: Test with pytest run: | pytest -v From bd3307ad428025e72fb66a9ca4e8231aa0917f9e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Sep 2023 16:29:43 -0400 Subject: [PATCH 03/32] Make "Update PyPA packages" step clearer --- .github/workflows/pythonpackage.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 696057ae2..a311798e2 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -50,12 +50,12 @@ jobs: - name: Update PyPA packages run: | - python -m pip install --upgrade pip + python -m pip install --upgrade pip wheel + + # Python prior to 3.12 ships setuptools. Upgrade it if present. if pip freeze --all | grep --quiet '^setuptools=='; then - # Python prior to 3.12 ships setuptools. Upgrade it if present. python -m pip install --upgrade setuptools fi - python -m pip install --upgrade wheel - name: Install project and test dependencies run: | From 680d7957373c6bf193388907e3dbb770f3867ffe Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 13 Sep 2023 17:13:01 -0400 Subject: [PATCH 04/32] Show all the failures Don't stop after the first 10. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index fa06458eb..0466ed4c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [tool.pytest.ini_options] python_files = 'test_*.py' testpaths = 'test' # space separated list of paths from root e.g test tests doc/testing -addopts = '--cov=git --cov-report=term --maxfail=10 --force-sugar --disable-warnings' +addopts = '--cov=git --cov-report=term --force-sugar --disable-warnings' filterwarnings = 'ignore::DeprecationWarning' # --cov coverage # --cov-report term # send report to terminal term-missing -> terminal with line numbers html xml From 75cf5402d0d6c6712c1b0f5bd114cc9fd8780edc Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Sep 2023 01:39:33 -0400 Subject: [PATCH 05/32] Keep sugar for local use, but use instafail on CI There are two benefits of the pytest-sugar plugin: 1. Pretty output. 2. Show details on each failure immediately instead of at the end. The first benefit is effectively local-only, because extra newlines are appearing when it runs on CI, both with and without -v. The second benefit applies both locally and on CI. So this adds the pytest-instafail plugin and uses it on CI to get the second benefit. It is not set up to run automatically, and pytest-sugar still is (though no longer forced), so local testing retains no benefit and we don't have a clash. The name "instafail" refers only to instantly *seeing* failures: it does not cause the pytest runner to stop earlier than otherwise. --- .github/workflows/cygwin-test.yml | 2 +- .github/workflows/pythonpackage.yml | 2 +- pyproject.toml | 2 +- test-requirements.txt | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 1563afc95..cae828099 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -64,4 +64,4 @@ jobs: - name: Test with pytest run: | set +x - /usr/bin/python -m pytest -v + /usr/bin/python -m pytest -p no:sugar -v --instafail diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index a311798e2..9ac1088f7 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -78,7 +78,7 @@ jobs: - name: Test with pytest run: | - pytest -v + pytest -v -p no:sugar --instafail continue-on-error: false - name: Documentation diff --git a/pyproject.toml b/pyproject.toml index 0466ed4c4..f4fc33fec 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [tool.pytest.ini_options] python_files = 'test_*.py' testpaths = 'test' # space separated list of paths from root e.g test tests doc/testing -addopts = '--cov=git --cov-report=term --force-sugar --disable-warnings' +addopts = '--cov=git --cov-report=term --disable-warnings' filterwarnings = 'ignore::DeprecationWarning' # --cov coverage # --cov-report term # send report to terminal term-missing -> terminal with line numbers html xml diff --git a/test-requirements.txt b/test-requirements.txt index b00dd6f06..1c08c736f 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -5,4 +5,5 @@ mypy pre-commit pytest pytest-cov +pytest-instafail pytest-sugar From eb56e7bdf15344739d3c2d671c1ca7dc185b8abe Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Sep 2023 02:01:25 -0400 Subject: [PATCH 06/32] Pass -v twice to see full skip reasons + Reorder pytest arguments so both workflows are consistent. --- .github/workflows/cygwin-test.yml | 2 +- .github/workflows/pythonpackage.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index cae828099..8d1145e76 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -64,4 +64,4 @@ jobs: - name: Test with pytest run: | set +x - /usr/bin/python -m pytest -p no:sugar -v --instafail + /usr/bin/python -m pytest -p no:sugar --instafail -vv diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 9ac1088f7..c0402e4bb 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -78,7 +78,7 @@ jobs: - name: Test with pytest run: | - pytest -v -p no:sugar --instafail + pytest -p no:sugar --instafail -vv continue-on-error: false - name: Documentation From 9c7ff1e4918128ff28ba02cb2771b440a392644c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Sep 2023 02:04:52 -0400 Subject: [PATCH 07/32] Force pytest color output on CI While pytest-sugar output gets mangled with extra newlines on CI, colorized output seems to work fine and improves readability. --- .github/workflows/cygwin-test.yml | 2 +- .github/workflows/pythonpackage.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 8d1145e76..337c0a809 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -64,4 +64,4 @@ jobs: - name: Test with pytest run: | set +x - /usr/bin/python -m pytest -p no:sugar --instafail -vv + /usr/bin/python -m pytest --color=yes -p no:sugar --instafail -vv diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index c0402e4bb..392c894bc 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -78,7 +78,7 @@ jobs: - name: Test with pytest run: | - pytest -p no:sugar --instafail -vv + pytest --color=yes -p no:sugar --instafail -vv continue-on-error: false - name: Documentation From 0eb38bcedf3703c2e3aacae27ea4cbafce33e941 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 15 Sep 2023 06:33:10 -0400 Subject: [PATCH 08/32] Fix test_blocking_lock_file for cygwin This permits the longer delay in test_blocking_lock_file--which was already allowed for native Windows--on Cygwin, where it is also needed. That lets the xfail mark for Cygwin be removed. This also updates the comments to avoid implying that the need for the delay is AppVeyor-specific (it seems needed on CI and locally). --- test/test_util.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index 42edc57cf..308ba311b 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -12,7 +12,6 @@ from unittest import mock, skipIf from datetime import datetime -import pytest import ddt from git.cmd import dashify @@ -156,11 +155,6 @@ def test_lock_file(self): lock_file._obtain_lock_or_raise() lock_file._release_lock() - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin fails here for some reason, always", - raises=AssertionError, - ) def test_blocking_lock_file(self): my_file = tempfile.mktemp() lock_file = BlockingLockFile(my_file) @@ -173,9 +167,8 @@ def test_blocking_lock_file(self): self.assertRaises(IOError, wait_lock._obtain_lock) elapsed = time.time() - start extra_time = 0.02 - if is_win: - # for Appveyor - extra_time *= 6 # NOTE: Indeterministic failures here... + if is_win or sys.platform == "cygwin": + extra_time *= 6 # NOTE: Indeterministic failures without this... self.assertLess(elapsed, wait_time + extra_time) def test_user_id(self): From 715dba473a202ef3631b6c4bd724b8ff4e6c6d0b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Sep 2023 02:51:58 -0400 Subject: [PATCH 09/32] Run cygpath tests on Cygwin, not native Windows They were not running on Cygwin, because git.util.is_win is False on Cygwin. They were running on native Windows, with a number of them always failing; these failures had sometimes been obscured by the --maxfail=10 that had formerly been used (from pyproject.toml). Many of them (not all the same ones) fail on Cygwin, and it might be valuable for cygpath to work on other platforms, especially native Windows. But I think it still makes sense to limit the tests to Cygwin at this time, because all the uses of cygpath in the project are in code that only runs after a check that the platform is Cygwin. Part of that check, as it is implemented, explicitly excludes native Windows (is_win must be false). --- test/test_util.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index 308ba311b..41d874678 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -9,7 +9,7 @@ import sys import tempfile import time -from unittest import mock, skipIf +from unittest import mock, skipUnless from datetime import datetime import ddt @@ -84,14 +84,14 @@ def setup(self): "array": [42], } - @skipIf(not is_win, "Paths specifically for Windows.") + @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.idata(_norm_cygpath_pairs + _unc_cygpath_pairs) def test_cygpath_ok(self, case): wpath, cpath = case cwpath = cygpath(wpath) self.assertEqual(cwpath, cpath, wpath) - @skipIf(not is_win, "Paths specifically for Windows.") + @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.data( (r"./bar", "bar"), (r".\bar", "bar"), @@ -104,7 +104,7 @@ def test_cygpath_norm_ok(self, case): cwpath = cygpath(wpath) self.assertEqual(cwpath, cpath or wpath, wpath) - @skipIf(not is_win, "Paths specifically for Windows.") + @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.data( r"C:", r"C:Relative", @@ -117,7 +117,7 @@ def test_cygpath_invalids(self, wpath): cwpath = cygpath(wpath) self.assertEqual(cwpath, wpath.replace("\\", "/"), wpath) - @skipIf(not is_win, "Paths specifically for Windows.") + @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.idata(_norm_cygpath_pairs) def test_decygpath(self, case): wpath, cpath = case From d6a2d2807de99715ce85887b8992dbcafcefcee9 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Sep 2023 04:08:02 -0400 Subject: [PATCH 10/32] Mark some cygpath tests xfail Two of the groups of cygpath tests in test_util.py generate tests that fail on Cygwin. There is no easy way to still run, but xfail, just the specific tests that fail, because the groups of tests are generated with `@ddt` parameterization, but neither the unittest nor pytest xfail mechanisms interact with that. If `@pytest.mark.parametrized` were used, this could be done. But that does not work on methods of test classes that derive from unittest.TestCase, including those in this project that indirectly derive from it by deriving from TestBase. The TestBase base class cannot be removed without overhauling many tests, due to fixtures it provides such as rorepo. So this marks too many tests as xfail, but in doing so allows test runs to pass while still exercising and showing status on all the tests, allowing result changes to be observed easily. --- test/test_util.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_util.py b/test/test_util.py index 41d874678..f2135a272 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -13,6 +13,7 @@ from datetime import datetime import ddt +import pytest from git.cmd import dashify from git.compat import is_win @@ -84,6 +85,10 @@ def setup(self): "array": [42], } + @pytest.mark.xfail( + reason="Many return paths prefixed /proc/cygdrive instead.", + raises=AssertionError, + ) @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.idata(_norm_cygpath_pairs + _unc_cygpath_pairs) def test_cygpath_ok(self, case): @@ -91,6 +96,10 @@ def test_cygpath_ok(self, case): cwpath = cygpath(wpath) self.assertEqual(cwpath, cpath, wpath) + @pytest.mark.xfail( + reason=r'2nd example r".\bar" -> "bar" fails, returns "./bar"', + raises=AssertionError, + ) @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.data( (r"./bar", "bar"), From 881456bdceb61d51fa84ea286e6ca0e3587e8dc5 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Sep 2023 06:54:56 -0400 Subject: [PATCH 11/32] Run test_commit_msg_hook_success on more systems This changes a default Windows skip of test_commit_msg_hook_success to an xfail, and makes it more specific, expecting failure only when either bash.exe is unavailable (definitely expected) or when bash.exe is the WSL bash wrapper in System32, which fails for some reason even though it's not at all clear it ought to. This showcases the failures rather than skipping, and also lets the test pass on Windows systems where bash.exe is something else, including the Git Bash bash.exe that native Windows CI would use. --- test/test_index.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/test/test_index.py b/test/test_index.py index fba9c78ec..f4858a586 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -7,10 +7,14 @@ from io import BytesIO import os +import os.path as osp +from pathlib import Path from stat import S_ISLNK, ST_MODE +import shutil import tempfile from unittest import skipIf -import shutil + +import pytest from git import ( IndexFile, @@ -23,6 +27,7 @@ GitCommandError, CheckoutError, ) +from git.cmd import Git from git.compat import is_win from git.exc import HookExecutionError, InvalidGitRepositoryError from git.index.fun import hook_path @@ -34,15 +39,22 @@ from git.util import HIDE_WINDOWS_KNOWN_ERRORS, hex_to_bin from gitdb.base import IStream -import os.path as osp -from git.cmd import Git +HOOKS_SHEBANG = "#!/usr/bin/env sh\n" -from pathlib import Path -HOOKS_SHEBANG = "#!/usr/bin/env sh\n" +def _found_in(cmd, directory): + """Check if a command is resolved in a directory (without following symlinks).""" + path = shutil.which(cmd) + return path and Path(path).parent == Path(directory) + is_win_without_bash = is_win and not shutil.which("bash.exe") +is_win_with_wsl_bash = is_win and _found_in( + cmd="bash.exe", + directory=Path(os.getenv("WINDIR")) / "System32", +) + def _make_hook(git_dir, name, content, make_exec=True): """A helper to create a hook""" @@ -910,7 +922,11 @@ def test_pre_commit_hook_fail(self, rw_repo): else: raise AssertionError("Should have caught a HookExecutionError") - @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, "TODO: fix hooks execution on Windows: #703") + @pytest.mark.xfail( + is_win_without_bash or is_win_with_wsl_bash, + reason="Specifically seems to fail on WSL bash (in spite of #1399)", + raises=AssertionError, + ) @with_rw_repo("HEAD", bare=True) def test_commit_msg_hook_success(self, rw_repo): commit_message = "commit default head by Frèderic Çaufl€" From c6a586ab4817a9dcb8c8290a6a3e7071b1834f32 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Sep 2023 07:07:52 -0400 Subject: [PATCH 12/32] No longer skip test_index_mutation on Cygwin As it seems to be working now on Cygwin (maybe not native Windows). --- test/test_index.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/test/test_index.py b/test/test_index.py index f4858a586..06db3aedd 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -12,7 +12,6 @@ from stat import S_ISLNK, ST_MODE import shutil import tempfile -from unittest import skipIf import pytest @@ -27,16 +26,13 @@ GitCommandError, CheckoutError, ) -from git.cmd import Git from git.compat import is_win from git.exc import HookExecutionError, InvalidGitRepositoryError from git.index.fun import hook_path from git.index.typ import BaseIndexEntry, IndexEntry from git.objects import Blob -from test.lib import TestBase, fixture_path, fixture, with_rw_repo -from test.lib import with_rw_directory -from git.util import Actor, rmtree -from git.util import HIDE_WINDOWS_KNOWN_ERRORS, hex_to_bin +from test.lib import TestBase, fixture, fixture_path, with_rw_directory, with_rw_repo +from git.util import Actor, hex_to_bin, rmtree from gitdb.base import IStream HOOKS_SHEBANG = "#!/usr/bin/env sh\n" @@ -434,14 +430,6 @@ def _count_existing(self, repo, files): # END num existing helper - @skipIf( - HIDE_WINDOWS_KNOWN_ERRORS and Git.is_cygwin(), - """FIXME: File "C:\\projects\\gitpython\\git\\test\\test_index.py", line 642, in test_index_mutation - self.assertEqual(fd.read(), link_target) - AssertionError: '!\xff\xfe/\x00e\x00t\x00c\x00/\x00t\x00h\x00a\x00t\x00\x00\x00' - != '/etc/that' - """, - ) @with_rw_repo("0.1.6") def test_index_mutation(self, rw_repo): index = rw_repo.index From fc022304d2f4bc8770f75b0c5fc289dccab0ae5b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 24 Sep 2023 02:27:31 -0400 Subject: [PATCH 13/32] Report encoding error in test_add_unicode as error This makes the test explicitly error out, rather than skipping, if it appears the environment doesn't support encoding Unicode filenames. Platforms these days should be capable of that, and reporting it as an error lessens the risk of missing a bug in the test code (that method or a fixture) if one is ever introduced. Erroring out will also make it easier to see the details in the chained UnicodeDecodeError exception. This does not affect the behavior of GitPython itself. It only changes how a test reports an unusual condition that keeps the test\ from being usefully run. --- test/test_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_base.py b/test/test_base.py index b77c8117d..90e701c4b 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -7,7 +7,7 @@ import os import sys import tempfile -from unittest import SkipTest, skipIf +from unittest import skipIf from git import Repo from git.objects import Blob, Tree, Commit, TagObject @@ -126,7 +126,7 @@ def test_add_unicode(self, rw_repo): try: file_path.encode(sys.getfilesystemencoding()) except UnicodeEncodeError as e: - raise SkipTest("Environment doesn't support unicode filenames") from e + raise RuntimeError("Environment doesn't support unicode filenames") from e with open(file_path, "wb") as fp: fp.write(b"something") From 203da23e5fe2ae5a0ae13d0f9b2a276ae584ea7b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 24 Sep 2023 03:19:23 -0400 Subject: [PATCH 14/32] Add a few FIXMEs re: better use of xfail --- test/test_config.py | 1 + test/test_util.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index 481e129c6..f805570d5 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -100,6 +100,7 @@ def test_includes_order(self): # values must be considered as soon as they get them assert r_config.get_value("diff", "tool") == "meld" try: + # FIXME: Split this assertion out somehow and mark it xfail (or fix it). assert r_config.get_value("sec", "var1") == "value1_main" except AssertionError as e: raise SkipTest("Known failure -- included values are not in effect right away") from e diff --git a/test/test_util.py b/test/test_util.py index f2135a272..2b1e518ed 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -85,6 +85,7 @@ def setup(self): "array": [42], } + # FIXME: Mark only the /proc-prefixing cases xfail, somehow (or fix them). @pytest.mark.xfail( reason="Many return paths prefixed /proc/cygdrive instead.", raises=AssertionError, @@ -103,7 +104,7 @@ def test_cygpath_ok(self, case): @skipUnless(sys.platform == "cygwin", "Paths specifically for Cygwin.") @ddt.data( (r"./bar", "bar"), - (r".\bar", "bar"), + (r".\bar", "bar"), # FIXME: Mark only this one xfail, somehow (or fix it). (r"../bar", "../bar"), (r"..\bar", "../bar"), (r"../bar/.\foo/../chu", "../bar/chu"), From cf5f1dca139b809a744badb9f9740dd6d0a70c56 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 03:21:05 -0400 Subject: [PATCH 15/32] Report <2.5.1 in test_linked_worktree_traversal as error Although GitPython does not require git >=2.5.1 in general, and this does *not* change that, this makes the unavailability of git 2.5.1 or later an error in test_linked_worktree_traversal, where it is needed to exercise that test, rather than skipping the test. A few systems, such as CentOS 7, may have downstream patched versions of git that remain safe to use yet are numbered <2.5.1 and do not have the necesary feature to run this test. But by now, users of those systems likely anticipate that other software would rely on the presence of features added in git 2.5.1, which was released over 7 years ago. As such, I think it is more useful to give an error for that test, so the test's inability to be run on the system is clear, than to automatically skip the test, which is likely to go unnoticed. --- test/test_fun.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_fun.py b/test/test_fun.py index d76e189ed..f39955aa0 100644 --- a/test/test_fun.py +++ b/test/test_fun.py @@ -2,7 +2,6 @@ from stat import S_IFDIR, S_IFREG, S_IFLNK, S_IXUSR from os import stat import os.path as osp -from unittest import SkipTest from git import Git from git.index import IndexFile @@ -279,7 +278,7 @@ def test_linked_worktree_traversal(self, rw_dir): """Check that we can identify a linked worktree based on a .git file""" git = Git(rw_dir) if git.version_info[:3] < (2, 5, 1): - raise SkipTest("worktree feature unsupported") + raise RuntimeError("worktree feature unsupported (test needs git 2.5.1 or later)") rw_master = self.rorepo.clone(join_path_native(rw_dir, "master_repo")) branch = rw_master.create_head("aaaaaaaa") From 89232365fb0204c32d1f7c23b9eb39fe4401eb7f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 04:13:36 -0400 Subject: [PATCH 16/32] Change skipIf(not ...) to skipUnless(...) --- test/test_submodule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 0aa80e5ce..432c02686 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -7,7 +7,7 @@ import tempfile from pathlib import Path import sys -from unittest import mock, skipIf +from unittest import mock, skipIf, skipUnless import pytest @@ -1039,7 +1039,7 @@ def test_branch_renames(self, rw_dir): assert sm_mod.commit() == sm_pfb.commit, "Now head should have been reset" assert sm_mod.head.ref.name == sm_pfb.name - @skipIf(not is_win, "Specifically for Windows.") + @skipUnless(is_win, "Specifically for Windows.") def test_to_relative_path_with_super_at_root_drive(self): class Repo(object): working_tree_dir = "D:\\" From b198bf1e7d9c9d9db675c6c4e94d2f136a0a7923 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 05:43:00 -0400 Subject: [PATCH 17/32] Express known test_depth failure with xfail Rather than skipping, so it becomes known if the situation changes. --- test/test_submodule.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 432c02686..54b1e796b 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -1050,9 +1050,8 @@ class Repo(object): msg = '_to_relative_path should be "submodule_path" but was "%s"' % relative_path assert relative_path == "submodule_path", msg - @skipIf( - True, - "for some unknown reason the assertion fails, even though it in fact is working in more common setup", + @pytest.mark.xfail( + reason="for some unknown reason the assertion fails, even though it in fact is working in more common setup", ) @with_rw_directory def test_depth(self, rwdir): From cd175a598ed457833bc06adba776e2bbb1d9014b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 07:09:43 -0400 Subject: [PATCH 18/32] Remove no-effect `@skipIf` on test_untracked_files It looked like test_untracked_files was sometimes skipped, and specifically that it would be skipped on Cygwin. But the `@skipIf` on it had the condition: HIDE_WINDOWS_KNOWN_ERRORS and Git.is_cygwin() HIDE_WINDOWS_KNOWN_ERRORS can only ever be true if it is set to a truthy value directly (not an intended use as it's a "constant"), or on native Windows systems: no matter how the environment variable related to it is set, it's only checked if is_win, which is set by checking os.name, which is only "nt" on native Windows systems, not Cygwin. So whenever HIDE_WINDOWS_KNOWN_ERRORS is true Git.is_cygwin() will be false. Thus this condition is never true and the test was never being skipped anyway: it was running and passing on Cygwin. --- test/test_repo.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/test_repo.py b/test/test_repo.py index 15899ec50..fe3419e33 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -13,7 +13,7 @@ import pickle import sys import tempfile -from unittest import mock, skipIf, SkipTest, skip +from unittest import mock, SkipTest, skip import pytest @@ -42,7 +42,7 @@ ) from git.repo.fun import touch from test.lib import TestBase, with_rw_repo, fixture -from git.util import HIDE_WINDOWS_KNOWN_ERRORS, cygpath +from git.util import cygpath from test.lib import with_rw_directory from git.util import join_path_native, rmtree, rmfile, bin_to_hex @@ -764,16 +764,6 @@ def test_blame_accepts_rev_opts(self, git): self.rorepo.blame("HEAD", "README.md", rev_opts=["-M", "-C", "-C"]) git.assert_called_once_with(*expected_args, **boilerplate_kwargs) - @skipIf( - HIDE_WINDOWS_KNOWN_ERRORS and Git.is_cygwin(), - """FIXME: File "C:\\projects\\gitpython\\git\\cmd.py", line 671, in execute - raise GitCommandError(command, status, stderr_value, stdout_value) - GitCommandError: Cmd('git') failed due to: exit code(128) - cmdline: git add 1__��ava verb��ten 1_test _myfile 1_test_other_file - 1_��ava-----verb��ten - stderr: 'fatal: pathspec '"1__çava verböten"' did not match any files' - """, - ) @with_rw_repo("HEAD", bare=False) def test_untracked_files(self, rwrepo): for run, repo_add in enumerate((rwrepo.index.add, rwrepo.git.add)): From f38cc000fe4d51f0f9d1bdedec86f6fcdb57f359 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 07:32:46 -0400 Subject: [PATCH 19/32] Make 2 more too-low git version skips into errors In the tests only, and not in any way affecting the feature set or requirements of GitPython itself. This is similar to, and with the same reasoning as, cf5f1dc. --- test/test_repo.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_repo.py b/test/test_repo.py index fe3419e33..4257d8a47 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -13,7 +13,7 @@ import pickle import sys import tempfile -from unittest import mock, SkipTest, skip +from unittest import mock, skip import pytest @@ -1235,7 +1235,7 @@ def test_merge_base(self): def test_is_ancestor(self): git = self.rorepo.git if git.version_info[:3] < (1, 8, 0): - raise SkipTest("git merge-base --is-ancestor feature unsupported") + raise RuntimeError("git merge-base --is-ancestor feature unsupported (test needs git 1.8.0 or later)") repo = self.rorepo c1 = "f6aa8d1" @@ -1283,7 +1283,7 @@ def test_git_work_tree_dotgit(self, rw_dir): based on it.""" git = Git(rw_dir) if git.version_info[:3] < (2, 5, 1): - raise SkipTest("worktree feature unsupported") + raise RuntimeError("worktree feature unsupported (test needs git 2.5.1 or later)") rw_master = self.rorepo.clone(join_path_native(rw_dir, "master_repo")) branch = rw_master.create_head("aaaaaaaa") From 8fd56e78366470e0b07db48daf623b188e7245ea Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 08:03:58 -0400 Subject: [PATCH 20/32] Update test_root_module Windows skip reason The current cause of failure is different from what is documented in the skip reason. --- test/test_submodule.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 54b1e796b..4c087caa1 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -477,11 +477,11 @@ def test_base_bare(self, rwrepo): @skipIf( HIDE_WINDOWS_KNOWN_ERRORS, """ - File "C:\\projects\\gitpython\\git\\cmd.py", line 559, in execute - raise GitCommandNotFound(command, err) - git.exc.GitCommandNotFound: Cmd('git') not found due to: OSError('[WinError 6] The handle is invalid') - cmdline: git clone -n --shared -v C:\\projects\\gitpython\\.git Users\\appveyor\\AppData\\Local\\Temp\\1\\tmplyp6kr_rnon_bare_test_root_module - """, # noqa E501 + E PermissionError: + [WinError 32] The process cannot access the file because it is being used by another process: + 'C:\\Users\\ek\\AppData\\Local\\Temp\\non_bare_test_root_modulep0eqt8_r\\git\\ext\\gitdb' + -> 'C:\\Users\\ek\\AppData\\Local\\Temp\\non_bare_test_root_modulep0eqt8_r\\path\\prefix\\git\\ext\\gitdb' + """, ) @with_rw_repo(k_subm_current, bare=False) def test_root_module(self, rwrepo): From c1798f5ead60f74de422732d5229244d00325f24 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 08:06:45 -0400 Subject: [PATCH 21/32] Change test_root_module Windows skip to xfail And rewrite the reason to give more useful information. (The new reason also doesn't state the exception type, because that is now specified, and checked by pytest, by being passed as "raises".) --- test/test_submodule.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 4c087caa1..256eb7dbf 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -474,14 +474,13 @@ def test_base_bare(self, rwrepo): reason="Cygwin GitPython can't find submodule SHA", raises=ValueError, ) - @skipIf( + @pytest.mark.xfail( HIDE_WINDOWS_KNOWN_ERRORS, - """ - E PermissionError: - [WinError 32] The process cannot access the file because it is being used by another process: - 'C:\\Users\\ek\\AppData\\Local\\Temp\\non_bare_test_root_modulep0eqt8_r\\git\\ext\\gitdb' - -> 'C:\\Users\\ek\\AppData\\Local\\Temp\\non_bare_test_root_modulep0eqt8_r\\path\\prefix\\git\\ext\\gitdb' - """, + reason=( + '"The process cannot access the file because it is being used by another process"' + + " on first call to rm.update" + ), + raises=PermissionError, ) @with_rw_repo(k_subm_current, bare=False) def test_root_module(self, rwrepo): From ba567521a5a01b93420cab4021d5663af66231ae Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 08:16:05 -0400 Subject: [PATCH 22/32] Update test_git_submodules_and_add_sm_with_new_commit skip reason This is working on Cygwin, so that old reason no longer applies. (The test was not being skipped on Cygwin, and was passing.) It is not working on native Windows, due to a PermissionError from attempting to move a file that is still open (which Windows doesn't allow). That may have been the original native Windows skip reason, but the old AppVeyor CI link for it is broken or not public. This makes the reason clear, though maybe I should add more details. --- test/test_submodule.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 256eb7dbf..25b0daa01 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -750,13 +750,12 @@ def test_list_only_valid_submodules(self, rwdir): @skipIf( HIDE_WINDOWS_KNOWN_ERRORS, - """FIXME on cygwin: File "C:\\projects\\gitpython\\git\\cmd.py", line 671, in execute - raise GitCommandError(command, status, stderr_value, stdout_value) - GitCommandError: Cmd('git') failed due to: exit code(128) - cmdline: git add 1__Xava verbXXten 1_test _myfile 1_test_other_file 1_XXava-----verbXXten - stderr: 'fatal: pathspec '"1__çava verböten"' did not match any files' - FIXME on appveyor: see https://ci.appveyor.com/project/Byron/gitpython/build/1.0.185 - """, + """ + E PermissionError: + [WinError 32] The process cannot access the file because it is being used by another process: + 'C:\\Users\\ek\\AppData\\Local\\Temp\\test_git_submodules_and_add_sm_with_new_commitu6d08von\\parent\\module' + -> 'C:\\Users\\ek\\AppData\\Local\\Temp\\test_git_submodules_and_add_sm_with_new_commitu6d08von\\parent\\module_moved' + """ # noqa: E501, ) @with_rw_directory @_patch_git_config("protocol.file.allow", "always") From 8704d1b81ba080cd4baa74968ac4cf84a84e4cbe Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 08:22:32 -0400 Subject: [PATCH 23/32] Change test_git_submodules_and_add_sm_with_new_commit Windows skip to xfail And improve details. The xfail is only for native Windows, not Cygwin (same as the old skip was, and still via checking HIDE_WINDOWS_KNOWN_ERRORS). This change is analogous to the change in c1798f5, but for test_git_submodules_and_add_sm_with_new_commit rather than test_root_module. --- test/test_submodule.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/test_submodule.py b/test/test_submodule.py index 25b0daa01..72d7255b0 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -7,7 +7,7 @@ import tempfile from pathlib import Path import sys -from unittest import mock, skipIf, skipUnless +from unittest import mock, skipUnless import pytest @@ -748,14 +748,13 @@ def test_list_only_valid_submodules(self, rwdir): repo = git.Repo(repo_path) assert len(repo.submodules) == 0 - @skipIf( + @pytest.mark.xfail( HIDE_WINDOWS_KNOWN_ERRORS, - """ - E PermissionError: - [WinError 32] The process cannot access the file because it is being used by another process: - 'C:\\Users\\ek\\AppData\\Local\\Temp\\test_git_submodules_and_add_sm_with_new_commitu6d08von\\parent\\module' - -> 'C:\\Users\\ek\\AppData\\Local\\Temp\\test_git_submodules_and_add_sm_with_new_commitu6d08von\\parent\\module_moved' - """ # noqa: E501, + reason=( + '"The process cannot access the file because it is being used by another process"' + + " on first call to sm.move" + ), + raises=PermissionError, ) @with_rw_directory @_patch_git_config("protocol.file.allow", "always") From 1d6abdca134082bf0bce2e590a52d8c08bf04d9b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 08:34:54 -0400 Subject: [PATCH 24/32] Run the tests in test_tree on Windows This stops skipping them, as they are now working. --- test/test_tree.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/test_tree.py b/test/test_tree.py index e59705645..c5ac8d539 100644 --- a/test/test_tree.py +++ b/test/test_tree.py @@ -5,24 +5,14 @@ # the BSD License: https://opensource.org/license/bsd-3-clause/ from io import BytesIO -from unittest import skipIf from git.objects import Tree, Blob from test.lib import TestBase -from git.util import HIDE_WINDOWS_KNOWN_ERRORS import os.path as osp class TestTree(TestBase): - @skipIf( - HIDE_WINDOWS_KNOWN_ERRORS, - """ - File "C:\\projects\\gitpython\\git\\cmd.py", line 559, in execute - raise GitCommandNotFound(command, err) - git.exc.GitCommandNotFound: Cmd('git') not found due to: OSError('[WinError 6] The handle is invalid') - cmdline: git cat-file --batch-check""", - ) def test_serializable(self): # tree at the given commit contains a submodule as well roottree = self.rorepo.tree("6c1faef799095f3990e9970bc2cb10aa0221cf9c") @@ -51,14 +41,6 @@ def test_serializable(self): testtree._deserialize(stream) # END for each item in tree - @skipIf( - HIDE_WINDOWS_KNOWN_ERRORS, - """ - File "C:\\projects\\gitpython\\git\\cmd.py", line 559, in execute - raise GitCommandNotFound(command, err) - git.exc.GitCommandNotFound: Cmd('git') not found due to: OSError('[WinError 6] The handle is invalid') - cmdline: git cat-file --batch-check""", - ) def test_traverse(self): root = self.rorepo.tree("0.1.6") num_recursive = 0 From 5609faa5a1c22654dfc007f7bf229e1b08087aa8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 09:01:31 -0400 Subject: [PATCH 25/32] Add missing raises keyword for test_depth xfail I had forgotten to do this earlier when converting from skip to xfail. Besides consistency with the other uses of xfail in the test suite, the benefit of passing "raises" is that pytest checks that the failure gave the expected exception and makes it a non-expected failure if it didn't. --- test/test_submodule.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_submodule.py b/test/test_submodule.py index 72d7255b0..79ff2c5f2 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -1049,6 +1049,7 @@ class Repo(object): @pytest.mark.xfail( reason="for some unknown reason the assertion fails, even though it in fact is working in more common setup", + raises=AssertionError, ) @with_rw_directory def test_depth(self, rwdir): From ed95e8e72f6aa697c1ec69595335168e717da013 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 09:07:53 -0400 Subject: [PATCH 26/32] Consolidate test_repo module import statements --- test/test_repo.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/test_repo.py b/test/test_repo.py index 4257d8a47..364b895fb 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -41,10 +41,8 @@ UnsafeProtocolError, ) from git.repo.fun import touch -from test.lib import TestBase, with_rw_repo, fixture -from git.util import cygpath -from test.lib import with_rw_directory -from git.util import join_path_native, rmtree, rmfile, bin_to_hex +from git.util import bin_to_hex, cygpath, join_path_native, rmfile, rmtree +from test.lib import TestBase, fixture, with_rw_directory, with_rw_repo import os.path as osp From ceb4dd3a7e89ac281a6c0d4d815fc13c00cbdf9f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 10:33:23 -0400 Subject: [PATCH 27/32] Show more CI system information --- .github/workflows/cygwin-test.yml | 1 + .github/workflows/pythonpackage.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 337c0a809..d0be6bc39 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -55,6 +55,7 @@ jobs: - name: Show version and platform information run: | + /usr/bin/uname -a /usr/bin/git version /usr/bin/python --version /usr/bin/python -c 'import sys; print(sys.platform)' diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 392c894bc..0dc9d217a 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -63,6 +63,7 @@ jobs: - name: Show version and platform information run: | + uname -a git version python --version python -c 'import sys; print(sys.platform)' From 3276aac711186a5dd0bd74ba1be8bb6f4ad3d03a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 10:49:50 -0400 Subject: [PATCH 28/32] Use Cygwin's bash and git for more CI steps --- .github/workflows/cygwin-test.yml | 34 +++++++++++++++++------------ .github/workflows/pythonpackage.yml | 1 + 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index d0be6bc39..2269df0da 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -14,11 +14,13 @@ jobs: TEMP: "/tmp" defaults: run: - shell: bash.exe --noprofile --norc -exo pipefail -o igncr "{0}" + shell: C:\cygwin\bin\bash.exe --noprofile --norc -exo pipefail -o igncr "{0}" steps: - name: Force LF line endings - run: git config --global core.autocrlf input + run: | + git config --global core.autocrlf input + shell: bash - uses: actions/checkout@v4 with: @@ -29,9 +31,12 @@ jobs: with: packages: python39 python39-pip python39-virtualenv git + - name: Limit $PATH to Cygwin + run: echo 'C:\cygwin\bin' >"$GITHUB_PATH" + - name: Tell git to trust this repo run: | - /usr/bin/git config --global --add safe.directory "$(pwd)" + git config --global --add safe.directory "$(pwd)" - name: Prepare this repo for tests run: | @@ -39,30 +44,31 @@ jobs: - name: Further prepare git configuration for tests run: | - /usr/bin/git config --global user.email "travis@ci.com" - /usr/bin/git config --global user.name "Travis Runner" + git config --global user.email "travis@ci.com" + git config --global user.name "Travis Runner" # If we rewrite the user's config by accident, we will mess it up # and cause subsequent tests to fail cat test/fixtures/.gitconfig >> ~/.gitconfig - name: Update PyPA packages run: | - /usr/bin/python -m pip install --upgrade pip setuptools wheel + python -m pip install --upgrade pip setuptools wheel - name: Install project and test dependencies run: | - /usr/bin/python -m pip install ".[test]" + python -m pip install ".[test]" - name: Show version and platform information run: | - /usr/bin/uname -a - /usr/bin/git version - /usr/bin/python --version - /usr/bin/python -c 'import sys; print(sys.platform)' - /usr/bin/python -c 'import os; print(os.name)' - /usr/bin/python -c 'import git; print(git.compat.is_win)' + uname -a + command -v git python + git version + python --version + python -c 'import sys; print(sys.platform)' + python -c 'import os; print(os.name)' + python -c 'import git; print(git.compat.is_win)' - name: Test with pytest run: | set +x - /usr/bin/python -m pytest --color=yes -p no:sugar --instafail -vv + python -m pytest --color=yes -p no:sugar --instafail -vv diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 0dc9d217a..78d3ddf86 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -64,6 +64,7 @@ jobs: - name: Show version and platform information run: | uname -a + command -v git python git version python --version python -c 'import sys; print(sys.platform)' From 5d4097654c6498d56defcc98dc1611fbfba9b75a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 12:02:35 -0400 Subject: [PATCH 29/32] Try to work in all LF on Cygwin CI + Style tweak and comment to clarify the "Limit $PATH" step. --- .github/workflows/cygwin-test.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 2269df0da..b8f3efbba 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -9,7 +9,6 @@ jobs: fail-fast: false env: CHERE_INVOKING: 1 - SHELLOPTS: igncr TMP: "/tmp" TEMP: "/tmp" defaults: @@ -19,7 +18,7 @@ jobs: steps: - name: Force LF line endings run: | - git config --global core.autocrlf input + git config --global core.autocrlf false # Affects the non-Cygwin git. shell: bash - uses: actions/checkout@v4 @@ -32,11 +31,13 @@ jobs: packages: python39 python39-pip python39-virtualenv git - name: Limit $PATH to Cygwin - run: echo 'C:\cygwin\bin' >"$GITHUB_PATH" + run: | + echo 'C:\cygwin\bin' > "$GITHUB_PATH" # Overwrite it with just this. - - name: Tell git to trust this repo + - name: Special configuration for Cygwin's git run: | git config --global --add safe.directory "$(pwd)" + git config --global core.autocrlf false - name: Prepare this repo for tests run: | @@ -70,5 +71,4 @@ jobs: - name: Test with pytest run: | - set +x python -m pytest --color=yes -p no:sugar --instafail -vv From dda428640113d6a2c30225ea33f1387e784b7289 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Sep 2023 13:42:29 -0400 Subject: [PATCH 30/32] Consistent formatting style across all workflows --- .github/workflows/cygwin-test.yml | 3 +++ .github/workflows/lint.yml | 12 +++++++----- .github/workflows/pythonpackage.yml | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index b8f3efbba..3cca4dd5f 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -5,12 +5,15 @@ on: [push, pull_request, workflow_dispatch] jobs: build: runs-on: windows-latest + strategy: fail-fast: false + env: CHERE_INVOKING: 1 TMP: "/tmp" TEMP: "/tmp" + defaults: run: shell: C:\cygwin\bin\bash.exe --noprofile --norc -exo pipefail -o igncr "{0}" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 5e79664a8..2204bb792 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,8 +7,10 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v4 - with: - python-version: "3.x" - - uses: pre-commit/action@v3.0.0 + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v4 + with: + python-version: "3.x" + + - uses: pre-commit/action@v3.0.0 diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 78d3ddf86..e9ccdd566 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -10,8 +10,8 @@ permissions: jobs: build: - runs-on: ubuntu-latest + strategy: fail-fast: false matrix: @@ -20,6 +20,7 @@ jobs: - experimental: false - python-version: "3.12" experimental: true + defaults: run: shell: /bin/bash --noprofile --norc -exo pipefail {0} From 3007abc6d229bcfe6643963f648597b7e231ab3d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 26 Sep 2023 00:01:32 -0400 Subject: [PATCH 31/32] Remove the recently added "Limit $PATH" step I had put that step in the Cygwin workflow for purposes of experimentation, and it seemed to make clearer what is going on, but really it does the opposite: it's deceptive because Cygwin uses other logic to set its PATH. So this step is unnecessary and ineffective at doing what it appears to do. --- .github/workflows/cygwin-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 3cca4dd5f..35e9fde72 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -33,10 +33,6 @@ jobs: with: packages: python39 python39-pip python39-virtualenv git - - name: Limit $PATH to Cygwin - run: | - echo 'C:\cygwin\bin' > "$GITHUB_PATH" # Overwrite it with just this. - - name: Special configuration for Cygwin's git run: | git config --global --add safe.directory "$(pwd)" From 4860f701b96dc07ac7c489c74c06cae069ae3cd1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 26 Sep 2023 00:14:29 -0400 Subject: [PATCH 32/32] Further reduce differences between test workflows This makes the two CI test workflows more similar in a couple of the remaining ways they differ unnecessarily. This could be extended, and otherwise improved upon, in the future. --- .github/workflows/cygwin-test.yml | 5 +++-- .github/workflows/pythonpackage.yml | 10 +++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 35e9fde72..e818803f1 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -42,7 +42,7 @@ jobs: run: | TRAVIS=yes ./init-tests-after-clone.sh - - name: Further prepare git configuration for tests + - name: Set git user identity and command aliases for the tests run: | git config --global user.email "travis@ci.com" git config --global user.name "Travis Runner" @@ -52,7 +52,8 @@ jobs: - name: Update PyPA packages run: | - python -m pip install --upgrade pip setuptools wheel + # Get the latest pip, wheel, and prior to Python 3.12, setuptools. + python -m pip install -U pip $(pip freeze --all | grep -oF setuptools) wheel - name: Install project and test dependencies run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index e9ccdd566..1b049ba02 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -41,7 +41,7 @@ jobs: run: | TRAVIS=yes ./init-tests-after-clone.sh - - name: Prepare git configuration for tests + - name: Set git user identity and command aliases for the tests run: | git config --global user.email "travis@ci.com" git config --global user.name "Travis Runner" @@ -51,12 +51,8 @@ jobs: - name: Update PyPA packages run: | - python -m pip install --upgrade pip wheel - - # Python prior to 3.12 ships setuptools. Upgrade it if present. - if pip freeze --all | grep --quiet '^setuptools=='; then - python -m pip install --upgrade setuptools - fi + # Get the latest pip, wheel, and prior to Python 3.12, setuptools. + python -m pip install -U pip $(pip freeze --all | grep -oF setuptools) wheel - name: Install project and test dependencies run: |