Repository navigation
test(minimal): T21 — Specs, types and entries inspected, end to end - #3338
minimal-foundry[bot] wants to merge 3 commits into
Conversation
One test scaffolds and appends entries with min init, lists and shows the types, renders entry and box specs over a test daemon, and checks that each render's span names the expansion it read.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Reviewed: no findings; approval withheld, 2 threads unresolved at APPROVE withheld: 2 unresolved threads — minimal-arch[bot] crates/minimal/src/tests.rs, minimal-foundry[bot] crates/minimal/src/tests.rs:18427. Resolving the last thread releases the approval on this head. Resolve the open threads. Resolving the last one releases the approval on this head; no new review is required. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none, since the diff only adds lines. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The test conforms for min init, min type and the stored-spec render. It also records a design gap: min box spec <entry> does not apply the Box Type defaults layer. Lines 1231 and 1281 of architecture.md put that layer in every expansion. The test comment calls the gap an interim, but no issue tracks it. Make the change in C1: file the issue, link it in the comment and the PR body, and state the flip assertion.
Ruled at 4e5e320 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| }; | ||
| std::fs::create_dir_all(project.path().join("config")).unwrap(); | ||
| // The type's own defaults (`timeout` here) are not asserted: the CLI | ||
| // expansion does not apply the type-defaults layer yet. |
There was a problem hiding this comment.
C1 · This comment records an interim with no issue. architecture.md lines 1231 and 1281 put the Box Type defaults layer in every expansion. File an issue for expand_entry and the start path, and link it here. Then state the flip: when the issue closes, add the timeout = "30m" needle and its layer label for web.
There was a problem hiding this comment.
Addressed in d256b3c6e67e: Rewrote the comment: it names the code that skips the type-defaults layer (expand_entry builds Layers::default()) and the web needle to add when it lands. I cannot file issues from this runner, so a person must file one and link it there.
| ] { | ||
| assert!( | ||
| log.lines() | ||
| .any(|line| line.contains("spec_type_init_end_to_end{") | ||
| && line.contains("verb=\"box spec\"") | ||
| && line.contains(&format!("{field}={value}")) | ||
| && line.contains("outcome=\"ok\"")), | ||
| "a `box spec` ok event for {field} {value} in the span\n{log}\n{ran}" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
MEDIUM · correctness · confidence 0.70 · in spec_type_init_end_to_end
The entry span field is a &str, so the fmt subscriber prints it quoted: entry="dev". The assertion searches for entry=dev without quotes, so the two entry cases will never match and the test will fail at the end. Only the box_id case (recorded with %) prints unquoted. Match entry="dev" for the string fields, or record entry with %.
What the reviewer checked
line.contains(&format!("{field}={value}")) with field "entry", value "dev"; compare tests.rs:9503 span format
box_id=\"{id}\"for a string field
Hand this to an agent
Act on a code-review finding.
File: crates/minimal/src/tests.rs
Lines: 18412-18422
Category: correctness
Severity: medium (reviewer confidence 0.70)
Finding:
The `entry` span field is a `&str`, so the fmt subscriber prints it quoted: `entry="dev"`. The assertion searches for `entry=dev` without quotes, so the two entry cases will never match and the test will fail at the end. Only the `box_id` case (recorded with `%`) prints unquoted. Match `entry="dev"` for the string fields, or record `entry` with `%`.
First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.
👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.
There was a problem hiding this comment.
Disputed — not addressed: The matched line is the ok event at cmd/box_spec.rs:596, which records entry = %rendered.entry, so it prints entry=dev unquoted. The test passes: cargo nextest run -p minimal spec_type_init_end_to_end, 1 passed.
…s-and-entries-inspected-end-to-end
Name the code that skips the Box Type defaults layer, and the needle to add for web when that layer lands.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Converted to a draft: something above needs a person before review. Review threads the coder judged:
One test in crates/minimal/src/tests.rs runs the whole flow against a test daemon. It scaffolds and appends entries with Notes from the agent: Revise round 1. Thread PRRT_kwDOPdfcxM622MBf asks for a filed issue. This box has no gh credential, so I could not file one. The test comment now names the gap ( |
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 2 unresolved threads — minimal-arch[bot] crates/minimal/src/tests.rs, minimal-foundry[bot] crates/minimal/src/tests.rs:18427. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The test is in the correct layer and proves min init, min type list and both min box spec forms. Two things block it. The Box Type defaults layer (architecture.md line 1231) is missing from the CLI expansion. The test calls this an interim, but no issue tracks it. File the issue and link it in the comment, as C1 says. The proof also says that min type show rust-dev names the source of each default and constraint, but the test asserts only the header. Add the needles in C2.
Ruled at d256b3c against gominimal/arch@15b603e6afb8 by claude-opus-5-5.
| ..daemon_global.clone() | ||
| }; | ||
| std::fs::create_dir_all(project.path().join("config")).unwrap(); | ||
| // Interim, with no tracking issue yet: architecture.md puts the Box Type |
There was a problem hiding this comment.
C1 · This interim has no issue. Architecture.m
D80D
d line 1231 puts the Box Type defaults layer in every expansion, and expand_entry omits it. File an issue for the layer. Replace "with no tracking issue yet" with the issue URL, and keep the flip condition.
| i32::from(shown.is_err()), | ||
| ); | ||
| let shown = shown.unwrap_or_else(|_| panic!("the type shows\n{ran}")); | ||
| for line in [ |
There was a problem hiding this comment.
C2 · The proof says this command names the source of each default and constraint (architecture.md line 463). These needles check only the header. Add the timeout = "30m" default with from rust-dev (project), and one constraint with from session (builtin).
T21 — Specs, types and entries inspected, end to end
slice: S5
kind: integration
covers: none
size: S
est: 20 code, 100 tests
executor: agent
difficulty: routine
model: light
diagnostics: when the end-to-end test fails, it prints each command it ran with that command's stdout, stderr and exit code
observability: the test run's spans show the expansion each render reads
layers:
- tests:
- crates/minimal/src/tests.rs: the slice's end-to-end test over a test daemon
test: cargo nextest run -p minimal spec_type_init_end_to_end
produces: none
consumes: none
proof: Render an entry's or a box's expanded spec with the layer and type of each value, list and show the types, and scaffold or append entries with
min init.depends on:
- blocked by #2906
- blocked by #2899
- blocked by #2893
Proof
In one test run over a test daemon, a person can see these steps. Bare min init scaffolds minimal.toml. min init --type appends the entries default (session), build (task) and web (the project type rust-dev). min type list shows the six built-in types and rust-dev with source project. min type show rust-dev names the source of each default and constraint. min box spec dev and min box spec web render the expanded spec with the entry's type in the header and the layer of each value, for example packages from [defaults.session]. min box spec -o json renders the spec stored with a box created on the daemon. The log shows each render inside the test span, with the entry or box id it read. On a failure, the test prints each command with its stdout, stderr and exit code.
Notes from the agent: Gaps found, outside this task's layers. The test asserts only what the branch delivers. (1) min box spec does not apply the type-defaults layer: expand_entry in crates/minimal/src/cmd/box_spec.rs builds Layers with no type_defaults. So the timeout = "30m" that [box_types.rust-dev.defaults] sets does not appear in min box spec web, although min type show rust-dev lists it. The label 'type defaults' is therefore never printed. The start path in crates/minimal/src/cmd/boxes/create.rs also does not apply the layer, so the render and the start agree. (2) min session start web, for an entry of a project type, is refused: the start cannot tell whether it is a session entry. (3) A session start by name takes only the entry 'default'. So the box form is proven with a box created from a 'default' entry that min init appends. The proof says 'the layer and type of each value'. The entry's type is in the render header, and the layer is beside each value. A per-value type label needs gap (1) fixed first. clippy -p minimal --all-targets reported nothing on the new lines.
Tests
cargo nextest run -p minimal spec_type_init_end_to_endVerify
min task run agent-verifyexited 100, so this PR is a draft.Closes #2914