GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — CI: vendor silent-error tests join vendor suite (#1089) by mikolalysenko · Pull Request #1550 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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.

All reactions
@claude
Start fix for #1089 (vendor silent slice)
0a232e2
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 14, 2026
Swarm health manager log #1382
Open
Claude (claude) added 2 commits October 14, 2026 22:02
@claude
Vendor silent staging tests join vendor suite
df12411
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
@claude
Pin vendor's real --silent error line
69c08f0
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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 14, 2026 22:07
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

cursor[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 14, 2026
Tracking: assert stable codes instead of human sentences in CLI tests, and fold the covgap suites #1089
Open
14 tasks
cursor[bot]
cursor Bot reviewed Oct 14, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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.

All reactions
@mikolalysenko Mikola Lysenko (mikolalysenko) changed the title CI: vendor silent-staging tests join vendor suite (#1089) CI: vendor silent-error tests join vendor suite (#1089) Oct 14, 2026
Tanmay182003
Tanmay Singla (Tanmay182003) approved these changes Oct 14, 2026
View reviewed changes
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 14, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit 17eecf7 Oct 14, 2026
86 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-vendor-silent-staging-errors branch October 14, 2026 23:56
This was referenced Oct 15, 2026
CI perf (settings): slow Depot 16-core hosts since 10-13 15:20 add ~1.5–2.5 min to the merge-queue critical path (~0.6k Linux job-min/day) #1512
Open
CI performance dashboard #1182
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@Tanmay182003 Tanmay Singla (Tanmay182003) Tanmay182003 approved these changes

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

Projects

None yet

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.

3 participants

@mikolalysenko @Tanmay182003 @claude
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.