test: add CLI end-to-end suite and a real external-metadata provider - #17
Merged
Conversation
Add a black-box end-to-end test target (scrap_e2e) that spawns the built scrap binary as a subprocess and asserts on argv -> exit code / stdout / stderr across the built-in, external, and project command paths. Implement MetadataProtocolProvider, a concrete ExternalMetadataProvider that probes `<cmd> --scrap-metadata` (falling back to `<cmd> --help`) via posix_spawn with a subprocess timeout, extracting a plain first-line description. Wire it into the composition root in place of the previous no-op provider and drop the now-unused NullMetadataProvider. Add direct unit tests for the provider (protocol, help fallback, timeout, missing/empty output).
Guarantee the metadata probe's timeout by unconditionally SIGKILLing the child's process group before the blocking waitpid, instead of only on the drain-timeout path. Capture-cap, EOF-while-still-running (e.g. a child that runs `exec 1>&-; sleep`), and drain errors previously fell through to an unbounded waitpid, letting a single misbehaving scrap-* freeze `scrap --help`. Killing an already-exited child is a harmless no-op on its zombie, so the happy-path exit status is preserved and WIFEXITED alone distinguishes a normal exit from a forced kill. Declare environ via <crt_externs.h>/_NSGetEnviron() on Apple, where <unistd.h> does not provide it, so the file builds on macOS. Wrap the pipe descriptors in a small RAII UniqueFd so every exit path closes them exactly once. Add regression tests for the capture-cap and closed-stdout hang variants (both must return within the injected timeout). Isolate the end-to-end metadata test to the --scrap-metadata path with a disjoint marker, create the e2e fixture dir atomically with mkdtemp, and label the e2e tests.
The unconditional SIGKILL before waitpid could discard a valid child's exit status: EOF on the pipe only proves stdout is closed, not that the child exited, so a probe that writes its line, closes stdout, then does brief work before exiting 0 would be killed mid-flight and reported as WIFSIGNALED, dropping the already-captured output. Reap with a bounded loop instead (extracted into reapBounded): a non-blocking waitpid reaps an already-exited child at once with its real status; a child still running is polled until the deadline so one that closed stdout early but exits shortly after is still reaped normally; only a child still alive at the deadline is SIGKILLed. Add a regression test for the early-stdout-close case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an end-to-end test suite that drives the built
scrapbinary as a realsubprocess, and replaces the placeholder external-metadata provider with a real
one that probes
scrap-*executables.Changes
MetadataProtocolProvider— a concreteExternalMetadataProviderthatruns
<cmd> --scrap-metadata(falling back to<cmd> --help) viaposix_spawnwith a bounded timeout and extracts a plain first-line description. It is wired
into the composition root in place of the previous no-op provider, and the
now-unused
NullMetadataProvideris removed. Structured JSON/options metadatais not part of the protocol yet.
scrap_e2e— a black-box GoogleTest target that spawns the built binary(
fork/execve) with a controlled environment and asserts onargv → exit code / stdout / stderr across the built-in, external, and project
command paths (labelled
e2e, soctest -L e2eselects them).--helpfallback, timeout,capture-cap and closed-stdout hang bounding, early-stdout-close, missing/empty).
Notes
misbehaving
scrap-*can never hang the CLI, while a provider that closes itsstdout before exiting still has its captured output preserved.
environis reached via_NSGetEnviron()on macOS.and clang-format clean.