From 3c91e7c3147af465176f56721a1dd02163e662e0 Mon Sep 17 00:00:00 2001 From: "benjamin.baker" Date: Sun, 20 Sep 2026 13:23:11 -0400 Subject: [PATCH] Resolve Git Bash explicitly, and skip on a missing privilege or tool Fifteen tests failed on this workstation and had been written off as environmental. Thirteen of them were not. Those thirteen ran subprocess.run(['bash', ...]). shutil.which('bash') finds System32\bash.exe - the WSL shim - which cannot execute a Windows path and failed every call with execvpe(/bin/bash). They now locate Git Bash from git.exe and probe it with a trivial script before trusting it, so a box with neither still skips rather than failing. The other two were genuinely environmental, but were skipping nothing and failing everything. The symlink test now skips only on WinError 1314, the absent SeCreateSymbolicLinkPrivilege; any other OSError still fails, because a symlink that cannot be created for some other reason is a real defect. The server certificate test skips when openssl is not installed, which is what the route shells out to. Being labelled environmental is why these sat for weeks. A test that cannot run should say so and skip; one that fails should mean something is broken. --- tests/test_naming_gate_migrations.py | 74 ++++++++++++++++--- tests/test_plugin_contract.py | 14 +++- .../test_endpoints_credentials.py | 4 + 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/tests/test_naming_gate_migrations.py b/tests/test_naming_gate_migrations.py index e02cfeee..ae9958d0 100644 --- a/tests/test_naming_gate_migrations.py +++ b/tests/test_naming_gate_migrations.py @@ -26,6 +26,56 @@ import pytest REPO_ROOT = Path(__file__).resolve().parent.parent GATE = REPO_ROOT / 'scripts' / 'check-naming-and-style.sh' + +def _find_bash(): + """Locate a bash that can actually run a POSIX script. + + shutil.which('bash') is not enough on Windows. The WSL shim at + System32\\bash.exe is on PATH by default, satisfies which(), and then fails + every invocation with + + WSL (...) ERROR: CreateProcessCommon:818: execvpe(/bin/bash) failed: + No such file or directory + + when no distribution is installed - because the path handed to it is a + Windows path that means nothing inside the WSL namespace. That is the same + trap CONTRIBUTING.md documents for running the gate by hand, and it is why + these 13 tests were recorded as "environmental failures" rather than being + fixed: they were reporting the shim, not the gate. + + Git Bash is preferred explicitly, then any candidate is proven by running a + trivial script through it before being trusted. + """ + candidates = [] + git_bash = shutil.which('git') + if git_bash: + # /cmd/git.exe -> /bin/bash.exe + candidates.append(str(Path(git_bash).resolve().parent.parent / 'bin' + / 'bash.exe')) + candidates.extend([ + r'C:\Program Files\Git\bin\bash.exe', + r'C:\Program Files (x86)\Git\bin\bash.exe', + ]) + found = shutil.which('bash') + if found: + candidates.append(found) + + for candidate in candidates: + if not candidate or not Path(candidate).exists(): + continue + try: + probe = subprocess.run([candidate, '-c', 'exit 0'], + capture_output=True, text=True, timeout=30) + except (OSError, subprocess.SubprocessError): + continue + if probe.returncode == 0 and 'execvpe' not in (probe.stderr or ''): + return candidate + return None + + +BASH = _find_bash() +needs_bash = pytest.mark.skipif(BASH is None, reason='needs a working bash') + # Same shapes the gate matches: the name literal of a created column, positional # or keyword, on the sa.Column( line or on the line after it, plus the target of # a rename. @@ -98,7 +148,7 @@ def test_no_migration_creates_a_badly_named_column(): + '\n '.join(offenders)) -@pytest.mark.skipif(shutil.which('bash') is None, reason='needs bash') +@needs_bash def test_the_gate_fails_on_a_planted_migration_column(tmp_path): """Plant one, and require the gate to say so. @@ -116,20 +166,20 @@ def test_the_gate_fails_on_a_planted_migration_column(tmp_path): planted = versions / 'zz01_planted.py' planted.write_text(PLANTED, encoding='utf-8') - dirty = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert dirty.returncode == 1, ( 'the gate passed a snake_case migration column:\n' + dirty.stdout) assert 'planted_column' in dirty.stdout planted.unlink() - clean = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert clean.returncode == 0, ( 'the gate fails with nothing planted:\n' + clean.stdout) -@pytest.mark.skipif(shutil.which('bash') is None, reason='needs bash') +@needs_bash def test_the_gate_fails_on_a_site_literal_in_the_agent(tmp_path): """agent/ is in the ADR-015 scan, and has to stay in it. @@ -146,14 +196,14 @@ def test_the_gate_fails_on_a_site_literal_in_the_agent(tmp_path): planted = agent / 'Probe.cs' planted.write_text(PLANTED_AGENT, encoding='utf-8') - dirty = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert dirty.returncode == 1, ( 'the gate passed a site hostname in the agent:\n' + dirty.stdout) assert 'Probe.cs' in dirty.stdout planted.unlink() - clean = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert clean.returncode == 0, ( 'the gate fails with nothing planted:\n' + clean.stdout) @@ -224,7 +274,7 @@ def _gate_tree(tmp_path): return gate -@pytest.mark.skipif(shutil.which('bash') is None, reason='needs bash') +@needs_bash @pytest.mark.parametrize('shape', sorted(PLANTED_COLUMN_SHAPES)) def test_the_gate_catches_every_column_shape(tmp_path, shape): """Plant one shape at a time and require the gate to name it. @@ -240,7 +290,7 @@ def test_the_gate_catches_every_column_shape(tmp_path, shape): planted = versions / 'zz01_planted.py' planted.write_text(_migration_module(body), encoding='utf-8') - dirty = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + dirty = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert dirty.returncode == 1, ( 'the gate passed a {0} column:\n{1}'.format(shape, dirty.stdout)) @@ -249,7 +299,7 @@ def test_the_gate_catches_every_column_shape(tmp_path, shape): expected, dirty.stdout)) -@pytest.mark.skipif(shutil.which('bash') is None, reason='needs bash') +@needs_bash def test_the_gate_passes_well_named_columns(tmp_path): """The other half: a gate that fails on everything is no better. @@ -260,13 +310,13 @@ def test_the_gate_passes_well_named_columns(tmp_path): gate = _gate_tree(tmp_path) (versions / 'zz01_clean.py').write_text( _migration_module(PLANTED_CLEAN_BODY), encoding='utf-8') - clean = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + clean = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert clean.returncode == 0, ( 'the gate failed on lowercase concatenated names:\n' + clean.stdout) -@pytest.mark.skipif(shutil.which('bash') is None, reason='needs bash') +@needs_bash def test_the_site_literal_scan_honours_its_include_list(tmp_path): """The ADR-015 scan must read the file types it declares, and no others. @@ -289,7 +339,7 @@ def test_the_site_literal_scan_honours_its_include_list(tmp_path): (agent / name).write_text( 'shipped to ' + PLANTED_SITE_HOST + '\n', encoding='utf-8') - result = subprocess.run(['bash', str(gate)], cwd=str(tmp_path), + result = subprocess.run([BASH, str(gate)], cwd=str(tmp_path), capture_output=True, text=True) assert result.returncode == 0, ( 'the scan read a file type it does not declare:\n' + result.stdout) diff --git a/tests/test_plugin_contract.py b/tests/test_plugin_contract.py index 2ea0f1cf..3da099d8 100644 --- a/tests/test_plugin_contract.py +++ b/tests/test_plugin_contract.py @@ -297,7 +297,19 @@ def test_import_scan_covers_symlinked_plugins(tmp_path): (external / 'plugin.py').write_text('from shopdb.extensions import db\n') link = root / 'zzsymlinkdemo' - link.symlink_to(external, target_is_directory=True) + try: + link.symlink_to(external, target_is_directory=True) + except OSError as error: + # WinError 1314 - SeCreateSymbolicLinkPrivilege is not held. On Windows + # that privilege belongs to Administrators, or to any account when + # Developer Mode is on, so an ordinary developer shell cannot create the + # fixture this test needs. Skip rather than fail: the scanner behaviour + # under test is real and is exercised on CI, and a red test here says + # nothing about the code. Only this specific cause is skipped - any + # other OSError still fails. + if getattr(error, 'winerror', None) == 1314: + pytest.skip('needs symlink privilege (Developer Mode or admin)') + raise try: scanned = {str(p) for p in _plugin_source_files()} assert any('symlinkdemo' in s for s in scanned), ( diff --git a/tests/test_plugins/test_endpoints_credentials.py b/tests/test_plugins/test_endpoints_credentials.py index ed4f8f3d..3b6ce9de 100644 --- a/tests/test_plugins/test_endpoints_credentials.py +++ b/tests/test_plugins/test_endpoints_credentials.py @@ -7,6 +7,8 @@ different PC is refused (ASSET_MISMATCH), the shared key is refused for that PC (CREDENTIAL_INVALID) so the agent knows to enroll again. """ +import shutil + import pytest from shopdb.core.models import Asset, AssetType, AuditLog, Setting @@ -882,6 +884,8 @@ def test_approve_refuses_an_enrolled_computer(client, db, app, keyheaders, # -- server certificate pin -------------------------------------------------- +@pytest.mark.skipif(shutil.which('openssl') is None, + reason='needs the openssl binary on PATH') def test_server_cert_reports_the_spki_pin(client, db, auth_headers, tmp_path, monkeypatch): import hashlib