Skip to content

Commit

Permalink
Improve environment isolation in Cygwin and Alpine Linux on CI
Browse files Browse the repository at this point in the history
The main change here is to the Cygwin test workflow, which now runs
the tests in a venv (a Python virtual environment), to avoid mixing
up our intended dependencies with other files installed by Python
related packages on the system. This should either fix the problem
where `test_handle_process_output` reports an extra line in stderr
for the `cat_file.py` subprocess on CI or Cygwin, or at least make
it somewhat easier to continue investigating the problem. I think
this change is also valuable for its own sake.

The connection to the `test_handle_process_output` failure is that
the extra line in stderr appears at the beginning and is an error
message about a missing import needed for subprocess code coverage.
With the recently added more detailed error reporting, it shows:

            self.assertEqual(line_counts[1], expected_line_count, repr(lines[1]))
    >       self.assertEqual(line_counts[2], expected_line_count, repr(lines[2]))
    E       AssertionError: 5003 != 5002 : ['pytest-cov: Failed to setup subprocess coverage. Environ: {\'COV_CORE_SOURCE\': \'git\', \'COV_CORE_CONFIG\': \':\', \'COV_CORE_DATAFILE\': \'/cygdrive/d/a/GitPython/GitPython/.coverage\'} Exception: ModuleNotFoundError("No module named \'iniconfig\'")\n', 'From github.com:jantman/gitpython_issue_301\n', ' = [up to date]      master     -> origin/master\n', ' = [up to date]      testcommit1 -> origin/testcommit1\n', ' = [up to date]      testcommit10 -> origin/testcommit10\n', ' = [up to date]      testcommit100 -> origin/testcommit100\n', ' = [up to date]      testcommit1000 -> origin/testcommit1000\n', ' = [up to date]      testcommit1001 -> origin/testcommit1001\n', ' = [up to date]      testcommit1002 -> origin/testcommit1002\n', ' = [up to date]      testcommit1003 -> origin/testcommit1003\n', ' = [up to date]      testcommit1004 -> origin/testcommit1004\n', ' = [up to date]      testcommit1005 -> origin/testcommit1005\n', ' = [up to date]      testcommit
    test/test_git.py:793: AssertionError

This could possibly be worked around by attempting to install a
package to provide `iniconfig`, by configuring `pytest-cov` in a
way that does not require it, or by temporarily disabling code
coverage reports on Cygwin. Before exploring those or other
options, this change seeks to prepare a more isolated environment
in which different package versions are more likely to work
properly together.

In addition to that change to the Cygwin workflow, this also
changes the way the Alpine Linux test workflow is made to use a
venv, using the technique that is used here, and undoing some
changes in the Alpine Linux workflow that should not be necessary.
The reason for making that change together with this Cygwin change
is that if either does not work as expected, it might shed light on
what is going wrong with the other.

Although the same technique is used on Cygwin and in Alpine Linux,
it looks a little different, because the environment variable on
Cygwin is `BASH_ENV` (since script steps are run in `bash`), while
the environment variable is `ENV` (since script steps are run in
`busybox sh`) and this must also be allowed to pass through `sudo`
(since `sh`, which is `busybox sh` on this system, is being invoked
through `sudo`).
  • Loading branch information
EliahKagan committed Jan 2, 2025
1 parent a0f8425 commit 4aad37a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
15 changes: 8 additions & 7 deletions .github/workflows/alpine-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ jobs:
steps:
- name: Prepare Alpine Linux
run: |
apk add sudo git git-daemon python3 py3-pip py3-setuptools py3-virtualenv py3-wheel
echo 'Defaults env_keep += "CI GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env
apk add sudo git git-daemon python3 py3-pip py3-virtualenv
echo 'Defaults env_keep += "CI ENV GITHUB_* RUNNER_*"' >/etc/sudoers.d/ci_env
addgroup -g 127 docker
adduser -D -u 1001 runner # TODO: Check if this still works on GHA as intended.
adduser runner docker
Expand Down Expand Up @@ -47,17 +47,19 @@ jobs:
- name: Set up virtualenv
run: |
python -m venv .venv
. .venv/bin/activate
printf '%s=%s\n' 'PATH' "$PATH" 'VIRTUAL_ENV' "$VIRTUAL_ENV" >>"$GITHUB_ENV"
echo 'ENV=.venv/bin/activate' >> "$GITHUB_ENV" # ENV (not BASH_ENV) for BusyBox sh.
- name: Update PyPA packages
run: |
# Get the latest pip, wheel, and prior to Python 3.12, setuptools.
python -m pip install -U pip $(pip freeze --all | grep -ow ^setuptools) wheel
- name: Install project and test dependencies
run: |
. .venv/bin/activate
pip install ".[test]"
- name: Show version and platform information
run: |
. .venv/bin/activate
uname -a
command -v git python
git version
Expand All @@ -66,5 +68,4 @@ jobs:
- name: Test with pytest
run: |
. .venv/bin/activate
pytest --color=yes -p no:sugar --instafail -vv
6 changes: 3 additions & 3 deletions .github/workflows/cygwin-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ jobs:
# and cause subsequent tests to fail
cat test/fixtures/.gitconfig >> ~/.gitconfig
- name: Ensure the "pip" command is available
- name: Set up virtualenv
run: |
# This is used unless, and before, an updated pip is installed.
ln -s pip3 /usr/bin/pip
python -m venv .venv
echo 'BASH_ENV=.venv/bin/activate' >>"$GITHUB_ENV"
- name: Update PyPA packages
run: |
Expand Down

0 comments on commit 4aad37a

Please sign in to comment.