Skip to content

fix(#266 Bucket B): reset server+DB between e2e suites — recover V3 Networks visibility (+19 tests) #273

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.

Sign up for GitHub

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
s2agi merged 1 commit into from
Jun 28, 2026
Merged

fix(#266 Bucket B): reset server+DB between e2e suites — recover V3 Networks visibility (+19 tests) #273

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 33 additions & 3 deletions tests/test-all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,43 @@ run_suite() {
echo ""
}

# Start server once
cd /app/server && bun run src/index.ts &>/dev/null &
sleep 3
# Each suite gets a fresh server + fresh DB. Without this, Base E2E leaves
# 137 tests' worth of state in commhub.db + binds :9200, which then breaks
# V3 Networks (state pollution → Results line never produced → reported as
# "0 ran") and Config Priority (its own `bun run src/index.ts &` gets
# EADDRINUSE because the parent's server still owns :9200, so the suite
# proceeds against the stale shared server and stops on the first
# `anet node create` that needs fresh-DB state). See #266 round-1 audit.
#
# Suites that don't start their own server (Base E2E, V3 Auth) rely on
# reset_server() to put one up for them. Suites that DO start their own
# (V3 Networks, Config Priority) get a free :9200 to bind to.
reset_server() {
pkill -f 'bun.*src/index.ts' 2>/dev/null || true
# Wait for :9200 to actually free up — pkill is async; binding before
# the old process releases would re-trigger the EADDRINUSE we just fixed.
for _ in $(seq 1 30); do
if ! (exec 3<>/dev/tcp/127.0.0.1/9200) 2>/dev/null; then break; fi
exec 3>&- 2>/dev/null || true
sleep 0.25
done
Comment on lines +56 to +60

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Abort when the old server never releases the port

If the previous server takes longer than this loop to exit, or if another non-matching process owns :9200, the loop simply falls through and the function proceeds to delete the DB and start a new Bun process anyway. That new process will hit EADDRINUSE, while the later /health probe can still succeed against the stale server, so the next suite runs with the old in-memory/open DB state even though reset_server reports success. Track whether the port actually became free and return failure before deleting state or starting the replacement server.

Useful? React with 👍 / 👎.

rm -rf /root/.commhub /root/.anet 2>/dev/null || true
cd /app/server && bun run src/index.ts &>/dev/null &
for _ in $(seq 1 30); do
curl -sf http://127.0.0.1:9200/health > /dev/null && return 0
sleep 0.5
done
echo "::warning::reset_server: server did not respond to /health within 15s" >&2
return 1
}

reset_server

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Stop when reset_server fails

When reset_server returns non-zero (for example, Bun exits or /health never becomes ready), the runner still invokes the suite because the return value is ignored. Several suite scripts start their own bun run src/index.ts & while run_suite captures stdout; if no parent server is actually up, that background server can stay alive and keep the command-substitution pipe open, turning a setup failure into a CI timeout instead of a clear failed reset. Gate each suite on a successful reset or count the reset failure immediately.

Useful? React with 👍 / 👎.

run_suite "Base E2E (137)" "/app/test.sh 2>&1"
reset_server
run_suite "V3 Auth (25)" "/app/test-auth.sh 2>&1"
reset_server
run_suite "V3 Networks (22)" "/app/test-networks.sh 2>&1"
reset_server
run_suite "Config Priority (16)" "/app/test-config.sh 2>&1"
Comment on lines +75 to 78

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not pre-bind 9200 for self-starting suites

For the V3 Networks and Config Priority suites, this reset starts a parent server on :9200 immediately before invoking scripts that also run cd /app/server && bun run src/index.ts & (tests/docker-e2e-networks.sh:13 and tests/docker-config-priority.sh:17). Those suite-local server starts now always hit EADDRINUSE, so the suites silently exercise the parent server instead of their own startup path or any per-suite server environment. Use a stop-and-wipe reset for self-starting suites, or remove the suite-local server starts so the ownership is unambiguous.

Useful? React with 👍 / 👎.


echo ""
Expand Down
Loading