Skip to content

Test native Windows on CI #1745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2fd79f4
Add native Windows test jobs to CI matrix
EliahKagan Oct 15, 2023
6e477e3
Add xfail marks for IndexFile.from_tree failures
EliahKagan Nov 14, 2023
cd9d7a9
Mark test_clone_command_injection xfail on Windows
EliahKagan Nov 15, 2023
f72e282
Mark test_diff_submodule xfail on Windows
EliahKagan Nov 15, 2023
42a3d74
Mark TestSubmodule.test_rename xfail on Windows
EliahKagan Nov 15, 2023
4abab92
Mark test_conditional_includes_from_git_dir xfail on Windows
EliahKagan Nov 15, 2023
799c853
Improve ordering/grouping of a few imports
EliahKagan Nov 16, 2023
b284ad7
Mark test_create_remote_unsafe_url_allowed xfail on Windows
EliahKagan Nov 16, 2023
61d1fba
Mark unsafe-options "allowed" tests xfail on Windows
EliahKagan Nov 16, 2023
ad07ecb
Show PATH on CI
EliahKagan Nov 22, 2023
2784e40
Show bash and other WSL-relevant info but not PATH
EliahKagan Nov 22, 2023
9717b8d
Install WSL system on CI for hook tests
EliahKagan Nov 17, 2023
5d11394
Fix and expand bash.exe xfail marks on hook tests
EliahKagan Nov 23, 2023
b215357
Simplify/clarify bash.exe check for hook tests; do it only once
EliahKagan Nov 24, 2023
cabb572
Temporarily don't install WSL system to test xfail
EliahKagan Nov 24, 2023
2875ffa
Put back WSL on Windows CI; pare down debug info
EliahKagan Nov 24, 2023
0f8cd4c
Treat XPASS status as a test failure
EliahKagan Nov 24, 2023
82c361e
Correct TestSubmodule.test_rename xfail condition
EliahKagan Nov 24, 2023
0ae5dd1
Revert "Treat XPASS status as a test failure"
EliahKagan Nov 24, 2023
0b7ee17
Refine TestSubmodule.test_rename xfail condition
EliahKagan Nov 25, 2023
8621e89
Reword comment in _WinBashStatus.check for clarity
EliahKagan Nov 25, 2023
7ff3cee
Make _WinBashStatus instances carry all their info
EliahKagan Nov 25, 2023
d5ed266
Use bytes in bash.exe check; retest no-distro case
EliahKagan Nov 25, 2023
496acaa
Handle multiple encodings for WSL error messages
EliahKagan Nov 26, 2023
d779a75
Don't assume WSL-related bash.exe error is English
EliahKagan Nov 27, 2023
9ac2438
Handle encodings better; make the sum type "public"
EliahKagan Nov 27, 2023
b07e5c7
Put back WSL on Windows CI
EliahKagan Nov 28, 2023
3303c74
Improve readability of WinBashStatus class
EliahKagan Nov 28, 2023
e00fffc
Shorten comments on _decode steps
EliahKagan Nov 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use bytes in bash.exe check; retest no-distro case
This removes text=True from the subprocess.run call, changing str
literals to bytes where appropriate and (less importantly) using
"%r" instead of "%s" in log messages so it's clear that printing
the repr of a bytes object is, at least for now, intentional.

The reason for this is that the encoding of error messages produced
by running the WSL bash.exe, when it attempts but fails to use a
WSL system, varies based on what error occurred. When no systems
are installed, the output can be decoded as UTF-8. When an error
from "deeper down" is reported, at least for Bash/Service errors,
the output is encoded in UTF-16LE, and attempting to decode it as
UTF-8 interleaves lots of null characters in the best case. With a
bytes object, loss of information is avoided, and it is clear on
inspection that the output requires decoding.

The most common case of such an error is *probably*:

    Insufficient system resources exist to complete the requested service.
    Error code: Bash/Service/CreateInstance/CreateVm/HCS/0x800705aa

However, that is tricky to produce intentionally on some systems.
To produce a test error, "wsl --shutdown" can be run repeatedly
while a _WinBashStatus.check() call is in progress. This produces:

    The virtual machine or container was forcefully exited.
    Error code: Bash/Service/0x80370107

Assuming the output always includes the text "Error code:", it may
be feasible to reliably detect which cases it is. This could
especially improve the log message. But for now that is not done.

In adddition to changing from text to binary mode, this commit also
temporarily removes the setup-wsl step from the CI workflow again,
to verify on CI that the text-to-binary change doesn't break the
WslNoDistro case. Manual testing shows the other cases still work.
  • Loading branch information
EliahKagan committed Nov 25, 2023
commit d5ed266f2208367e4300c29fa4cbda9cd2db0fb9
6 changes: 0 additions & 6 deletions .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ jobs:
python-version: ${{ matrix.python-version }}
allow-prereleases: ${{ matrix.experimental }}

- name: Set up WSL (Windows)
if: startsWith(matrix.os, 'windows')
uses: Vampire/setup-wsl@v2.0.2
with:
distribution: Debian

- name: Prepare this repo for tests
run: |
./init-tests-after-clone.sh
Expand Down
13 changes: 7 additions & 6 deletions test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ def check(cls):
if os.name != "nt":
return cls.Inapplicable()

no_distro_message = "Windows Subsystem for Linux has no installed distributions."
# Use bytes because messages for different WSL errors use different encodings.
no_distro_message = b"Windows Subsystem for Linux has no installed distributions."

def error_running_bash(error):
log.error("Error running bash.exe to check WSL status: %s", error)
log.error("Error running bash.exe to check WSL status: %r", error)
return cls.ErrorWhileChecking(error)

try:
Expand All @@ -106,7 +107,7 @@ def error_running_bash(error):
# information on ways to check for WSL, see https://superuser.com/a/1749811.
script = 'test -e /proc/sys/fs/binfmt_misc/WSLInterop; echo "$?"'
command = ["bash.exe", "-c", script]
proc = subprocess.run(command, capture_output=True, check=True, text=True)
proc = subprocess.run(command, capture_output=True, check=True)
except FileNotFoundError:
return cls.Absent()
except OSError as error:
Expand All @@ -117,11 +118,11 @@ def error_running_bash(error):
return error_running_bash(error)

status = proc.stdout.rstrip()
if status == "0":
if status == b"0":
return cls.Wsl()
if status == "1":
if status == b"1":
return cls.Native()
log.error("Strange output checking WSL status: %s", proc.stdout)
log.error("Strange output checking WSL status: %r", proc.stdout)
return cls.ErrorWhileChecking(proc)


Expand Down