Skip to content

fix: update test script to work without lsof on CI#798

Merged
jrvb-rl merged 2 commits into
mainfrom
jrvb/fix-lsof
Jun 26, 2026
Merged

fix: update test script to work without lsof on CI#798
jrvb-rl merged 2 commits into
mainfrom
jrvb/fix-lsof

Conversation

@jrvb-rl

@jrvb-rl jrvb-rl commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

We had one test which worked locally but failed reliably in the github CI environment due to lsof not being installed. This fix should get the CI tests back to green.

  • Add fallbacks for kill_server_on_port when lsof is unavailable (e.g. on GitHub Actions runners)
  • Try ss, then fuser, then parse /proc/net/tcp directly as a final fallback
  • Warn gracefully if no method is available

Test plan

  • Verify CI passes on GitHub Actions (Linux runners without lsof)
  • Verify local ./scripts/test still works on macOS (uses lsof path)

🤖 Generated with Claude Code

@jrvb-rl jrvb-rl changed the title Fix test script to work without lsof on CI fix: update test script to work without lsof on CI May 8, 2026
jrvb-rl and others added 2 commits June 26, 2026 09:59
Use ss or fuser as fallbacks when lsof is not available, and warn
gracefully if none are present.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
When lsof, ss, and fuser are all unavailable, parse /proc/net/tcp to
find socket inodes listening on the target port, then scan /proc/*/fd/
to map inodes back to pids. Always available on Linux.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@jrvb-rl jrvb-rl requested review from james-rl and sid-rl June 26, 2026 17:01

@james-rl james-rl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok. Maybe a bit overly defensive -- is there any point to trying lsof then using the fallbacks if we know lsof won't be available anyway?

Comment thread scripts/test
elif command -v fuser >/dev/null 2>&1; then
pids=$(fuser "$1/tcp" 2>/dev/null || echo "")
elif [ -d /proc/net ]; then
pids=$(find_pids_on_port_via_proc "$1")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just do this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it was helpful to just try all of the reasonable things, to reduce the chances of this breaking in the future in different environments. Eg, the /proc reading stuff is good for linux, but not for local runs on mac.

All of this is just used to make sure things are in a clean state when tests run and reduce flakes, so being a bit more robust seems like a good way to reduce our test flake noise.

@jrvb-rl jrvb-rl merged commit 50507eb into main Jun 26, 2026
7 checks passed
@jrvb-rl jrvb-rl deleted the jrvb/fix-lsof branch June 26, 2026 18:16
@stainless-app stainless-app Bot mentioned this pull request Jun 26, 2026
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.

2 participants