GeezerVison is showing github.com — CI: vendor silent-error tests join vendor suite (#1089) by mikolalysenko · Pull Request #1550 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Refs #1089. This is the last open piece of child 3: the vendor twin of the apply slice in #1546.
What changes
tests/coverage_fix_vendor_silent_mute_exit.rs was a standalone test binary. It is now tests/vendor/vendor_silent_errors.rs inside the vendor suite binary, so CI links one fewer integration test binary.
- The
--json test reads the envelope through common::envelope. It checks for a failed event with errorCode = apply_failed whose purl is the patch, instead of a raw events[0]["errorCode"] read.
- The two
--silent tests now check that stderr has an Error: line naming the patch purl. They used to match free-form words: offline, and request, which comes as much from reqwest's error text as from ours.
- The module docs now say what the tests actually pin: vendor's per-patch
Error: line, offline and against a dead patch service. The old docs named a stage_vendor_sources_in_memory staging layer and a no_local_source envelope code, and vendor uses neither. The apply twin's cross-reference is corrected to match.
dead_endpoint() moves into vendor/npm_fixture.rs. It had three identical copies in the vendor binary, and two of them now import it from there.
scripts/ci-test-durations.json (Windows shard timings only): the old binary's entry is dropped and vendor goes from 11.52s to 12.8s. That adds only the moved tests' runtime (~1.3s); the rest of the old 7.4s was the removed binary's link time.
CI cost
Workflows are unchanged. A lean PR run has the same ~28 Linux jobs before and after, at roughly the same job-minutes. The Linux test shards build one fewer test binary, which saves a few seconds of link time per shard. That is too small to justify a ci-perf label.
Test evidence
cargo test -p socket-patch-cli --all-features --test vendor: 151 passed, including the 3 moved tests.
cargo fmt --all -- --check: clean.
cargo clippy --locked --workspace --all-features -- -D warnings (CI's invocation): clean.
python3 -m unittest discover -s scripts/tests: 347 tests OK, 6 skipped. This includes the shard planner test that reads ci-test-durations.json.
- CI on 69c08f0:
ci-ok and clippy green, and every lean job passed. Cursor Bugbot found no issues.
/code-review high
I fixed these findings: the stale docs, the sentence-fragment matches, the offline run spawning an unneeded fixture patch service (it now uses hermetic::binary_command directly, as before), the staging misnomer in the file name, the questionable partialFailure assertion (dropped), the duplicated dead_endpoint, the apply twin's stale cross-reference, and the inflated shard timing.
I did not take these:
- Use the siblings'
sktsec_placeholder_value_for_tests_api token. That token triggers the token-shape warning on stderr, and the --json test requires stderr to stay clean. I tried it and the test failed, so the well-formed token stays, with a comment explaining why.
- Share
stderr_chatter through common. It has five copies across five different test binaries. Merging them is a cross-suite change outside this slice.
🤖 Generated with Claude Code
https://claude.ai/code/session_017QqpxAuxWR1QbGKykwjMUV
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
coverage_fix_vendor_silent_mute_exit.rs was its own test binary with
its own runner and a raw events[0] envelope read. It now lives in the
vendor suite as vendor/vendor_silent_staging_errors.rs, next to its
apply twin apply/apply_silent_staging_errors.rs, so CI links one fewer
test binary.
The tests spawn the CLI through npm_fixture::run_cli (hermetic env),
read the --json envelope through common::envelope (find_event for the
failed/apply_failed event, plus its purl and partialFailure status),
and the --silent checks require the error line to name the patch purl
rather than matching a free-form word. The module doc now names the
envelope code vendor really emits (apply_failed, not no_local_source).
The Windows shard timings move the old binary's seconds onto vendor.
Refs #1089.
Assisted-by: Claude Code:claude-opus-5-5
Review of the move found the vendor silent tests described a staging
layer vendor no longer uses and matched the free-form word "request",
which comes as much from the HTTP client's text as from ours.
The file is now vendor/vendor_silent_errors.rs. Its docs describe what
it really pins: the per-patch "Error:" line vendor prints for a patch
it can't source, offline and against a dead patch service. Both
--silent tests now require an "Error:" line naming the patch purl. The
offline run spawns the hermetic binary directly again, so no fixture
patch service or SOCKET_VENDOR_URL changes the branch under test.
dead_endpoint() moves into npm_fixture.rs instead of being copied a
third time into the vendor binary. The apply twin's docs no longer
claim vendor emits no_local_source, and the vendor shard timing counts
only the moved tests' runtime, not the old binary's link time.
Refs #1089.
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 69c08f0. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent)
- Head:
69c08f00427a880020f375c324f4f2723048927d
- CI: 86/86 check runs green on this head (36 success, 50 path-skipped); no main-wide failures.
- Bugbot: reviewed
69c08f0, no findings; no open review threads.
- Already approved by Tanmay Singla (@Tanmay182003) on this head. Test-only change (vendor silent-error tests moved into the vendor suite, asserting on error codes); no
CHANGELOG.md change.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 15, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|