8000 fix(minimald): add new ListenArgs fields to the test_cli initializer by norrietaylor · Pull Request #1757 · gominimal/minimal · GitHub
[go: up one dir, main page]
Skip to content

fix(minimald): add new ListenArgs fields to the test_cli initializer - #1757

Merged
norrietaylor merged 1 commit into
mainfrom
claude/clippy-ci-broken-main-8qyxyg
Sep 29, 2026
Merged

fix(minimald): add new ListenArgs fields to the test_cli initializer#1757
norrietaylor merged 1 commit into
mainfrom
claude/clippy-ci-broken-main-8qyxyg

Conversation

@norrietaylor
@norrietaylor norrietaylor commented Sep 29, 2026 •
edited by coderabbitai Bot
Loading
Copy link
Copy Markdown
Member

Summary

clippy --workspace --all-targets fails on main with E0063: the test_cli helper in crates/minimald/src/main.rs builds ListenArgs by hand and is missing hostname_proxy_port, zone_answerer_port and egress_deny_all_opt_out, so the minimald test target doesn't compile. This adds the three fields, with the same defaults the microVM init path uses (None, None, false).

How it started: a semantic merge conflict between three PRs, with no textual conflict to flag it.

The root gap is that the merge path doesn't re-validate a PR against the current base tip (no merge queue or require-up-to-date rule), so two individually green PRs can break main together. Enabling a merge queue or "require branches to be up to date" on the ruleset would prevent this class of failure. That's a repo-settings decision, so I haven't changed anything.

Testing

  • cargo clippy --workspace --al 8000 l-targets --locked -- -D warnings: clean (failed with E0063 before the change)
  • cargo fmt --check -p minimald: clean
  • Tests not run.

Checklist

  • Docs updated if behavior changed (n/a, test-only change)
  • BREAKING CHANGE: footer present if this is a breaking change (n/a)

🤖 Generated with Claude Code

https://claude.ai/code/session_01EgnhoZwtoZz6vFJZcfvZ3K

Generated by Claude Code

Summary by CodeRabbit

  • Tests
    • Updated command-line test setup to include the current listener configuration options. No user-facing behavior changes.

@claude
fix(minimald): add new ListenArgs fields to the test_cli initializer
93c2ecc 
The egress and proxy-port flags were added to ListenArgs without updating
the unit-test initializer, so the minimald test target failed to compile
and clippy --all-targets went red on main.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgnhoZwtoZz6vFJZcfvZ3K
@norrietaylor
norrietaylor requested a review from a team as a code owner September 29, 2026 05:45
@CLAassistant
CLAassistant commented Sep 29, 2026
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
coderabbitai Bot commented Sep 29, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: f49fc221-4fee-4aa7-993b-0524b68e3baf

📥 Commits

Reviewing files that changed from the base of the PR and between 6727d04 and 93c2ecc.

📒 Files selected for processing (1)
  • crates/minimald/src/main.rs

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

📝 Walkthrough

Walkthrough

The test_cli fixture now initializes the proxy-port options to None and the egress deny-all opt-out to false.

Changes

CLI Test Fixture Defaults

Layer / File(s) Summary
Set fixture defaults
crates/minimald/src/main.rs		 The test_cli fixture sets hostname_proxy_port and zone_answerer_port to None, and egress_deny_all_opt_out to false.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Bug fix

Suggested reviewers: evanspearman

Merge Risk: ⚪ Minimal · up to 93c2e

The fixture now supplies defaults for the added options, and no material merge-blocking risk is evident in the supplied scope.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required Conventional Commit format and clearly identifies the addition of the missing ListenArgs fields to the test_cli initializer.
Description check ✅ Passed The description includes complete Summary, Testing, and Checklist sections. It explains the cause, the change, the validation results, and why documentation and breaking-change updates do not apply.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the fixture with care,
Three defaults now wait in their place.
Two ports rest as None in the code,
One opt-out is false on the road.
Then off hops the rabbit, content and light.

Comment @coderabbitai help to get the list of available commands.

@minimal-foundry
minimal-foundry Bot commented Sep 29, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

Approved: no findings at 93c2ecc8cb34.

Nothing to do.

Clobber: none: git log on crates/minimald/src/main.rs confirms #1682 (test_cli) merged after the two field-adding PRs #1701 and #1727, matching the described semantic-merge-conflict root cause; nothing is reverted.

minimal-foundry[bot]
minimal-foundry Bot approved these changes Sep 29, 2026
View reviewed changes
@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minimal-review — approved

Verdict APPROVE — No findings.

@minimal-foundry minimal-foundry Bot added the review:approved minimal-review approved the current head label Sep 29, 2026
@norrietaylor
norrietaylor merged commit cb9d115 into main Sep 29, 2026
30 of 33 checks passed
@norrietaylor
norrietaylor deleted the claude/clippy-ci-broken-main-8qyxyg branch September 29, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mitodrummer mitodrummer mitodrummer approved these changes
@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes
@bryan-minimal bryan-minimal bryan-minimal approved these changes
@minimal-foundry minimal-foundry[bot] minimal-foundry[bot] approved these changes

Assignees

No one assigned

Labels

review:approved minimal-review approved the current head

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@norrietaylor @CLAassistant @mitodrummer @bryan-minimal @claude
0