Skip to content

test(e2e): add SIGTERM-mid-run abort coverage to test-api-e2e-cleanup-check.sh #1388

Description

@Dumbris

Context

PR #1368 (`fix: address final review of test-api-e2e.sh cleanup scoping`)
hardened `scripts/test-api-e2e.sh`'s `cleanup()` trap to reap only this
run's own processes, and its regression check
(`scripts/test-api-e2e-cleanup-check.sh`) now:

  • starts a real decoy `mcpproxy serve` core AND a launcher-fixture
    argv stand-in, and asserts both survive a full run of the suite;
  • asserts both decoys match the OLD blanket `pkill -f` patterns this
    PR removed, so the survival assertions actually prove something;
  • continuously samples (once per second) the full descendant tree of
    the suite's own core (and its audit-log sub-instance, if that
    sub-test runs) while the suite is in progress, and asserts none of
    those descendants outlive the run.

Gap

The check only exercises the suite's normal, complete-to-the-end
run. It does not cover the scenario where cleanup hygiene matters
most: the suite's own process is SIGTERMed mid-run, e.g. partway
through `test_launcher_lifecycle`, with the core, the launcher-test
fixture and the "everything" MCP server's npx child all still live.

A prior draft of the check attempted a two-mode ("complete" + "abort")
design, timed to interrupt the launcher-lifecycle test specifically.
It was simplified away during review because a correctness fix
shouldn't ship blocked on a flaky, timing-dependent second test mode
(see PR #1368 review discussion). This issue tracks adding it back
properly, as separate, focused follow-up work:

  • run the suite in the background, using a reliable marker (e.g. a
    fixed sleep after a specific log line, or a semaphore file the
    suite touches at a known point) rather than a raw wall-clock delay,
    to fire SIGTERM once the launcher-lifecycle test has started;
  • reuse the existing continuous descendant sampler (already added by
    PR fix(e2e): scope test-api-e2e.sh cleanup to this run's own processes #1368) to assert zero leaks after the abort, exactly as the
    complete-run path does today;
  • iterate on it locally until it is not flaky (run it back-to-back
    several times) before merging, since this script is not part of CI
    (it needs a real, slow, external-dependency-having E2E run) and a
    flaky abort mode would erode trust in the whole check.

Not blocking any release; this is a coverage improvement to a local,
manually-run regression check.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions