Skip to content

fix(tests): port-detection suite loaded no functions at all on macOS - #5001

Open
AniketBajaj2024 wants to merge 1 commit into
Osmantic:mainfrom
AniketBajaj2024:fix/port-test-gnu-sed-silent-noop
Open

fix(tests): port-detection suite loaded no functions at all on macOS#5001
AniketBajaj2024 wants to merge 1 commit into
Osmantic:mainfrom
AniketBajaj2024:fix/port-test-gnu-sed-silent-noop

Conversation

@AniketBajaj2024

Copy link
Copy Markdown
Contributor

Problem: Two compounding defects. The sed range used GNU \s, which BSD sed reads as a literal s — both ranges matched nothing, extracting an empty script. And source <(empty) exits 0, so the || { fail; exit 1; } guard never fired. The suite sailed past its own safety check and died later at line 100: PORT_CONFLICT: unbound variable. Directly contradicts CLAUDE.md's "POSIX-compatible constructs preferred for macOS portability (avoid GNU-only date/grep)".

Fix: POSIX [[:space:]] classes, plus an explicit per-function grep verification before sourcing, since the exit code cannot detect emptiness.

The function extraction used GNU sed's \s shorthand:

  sed -n '/^check_port_conflict\s*()\s*{/,/^}/p;...'

BSD sed (macOS) has no \s -- it matches a literal 's', so neither range
matches and the extraction returns an empty script. 'source <(...)' of an
empty script exits 0, so the '|| { echo FAIL; exit 1; }' guard never fired
and the suite ran on with no functions defined: every test failed with
'check_port_conflict: command not found', then died at line 100 with
'PORT_CONFLICT: unbound variable' under set -u, skipping the rest.

Switch to POSIX [[:space:]] classes (per CLAUDE.md: avoid GNU-only
constructs for macOS portability) and check the extracted text actually
contains both functions, since an empty source cannot be detected by exit
status alone.

Verified on macOS: suite went from aborting at test 4 with
'PORT_CONFLICT: unbound variable' to 'All tests passed (14/14)'.
Renaming one pattern so it matches nothing now fails loudly with
'Cannot load check_port_conflict' instead of sourcing an empty script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant