Skip to content

Commit eef6b43

Browse files
Duncanwpfleger96
andcommitted
fix(canvas): close three minor post-review cleanups
- Route verification-read-fails branch through injected out writer so cmd_restore_canvas captures all stdout; assert event_id/accepted/message in restore_succeeds_when_verification_read_fails. - Scope pending-guard alert query to the originating <li> and assert errorText contains the rejection message; narrow invariant-3 comments to match what the test actually exercises (row expand buttons, not the Restore action button). - Update conflict_relay doc block: attempt 1 returns retryable HTTP 503, not a dropped TCP connection. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
1 parent c5c08cc commit eef6b43

2 files changed

Lines changed: 35 additions & 13 deletions

File tree

crates/buzz-cli/src/commands/channels.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ pub async fn cmd_restore_canvas(
518518
eprintln!(
519519
"warning: canvas restore {our_id} was published but its post-write verification read failed; the restore is preserved in history — check `buzz canvas history` if a concurrent edit may have landed"
520520
);
521-
println!("{}", normalize_write_response(&resp));
521+
let _ = writeln!(out, "{}", normalize_write_response(&resp));
522522
return Ok(());
523523
}
524524
};
@@ -3819,13 +3819,28 @@ mod restore_canvas_tests {
38193819
#[tokio::test]
38203820
async fn restore_succeeds_when_verification_read_fails() {
38213821
let (url, submitted, _) = relay(PostHead::ReadFails).await;
3822-
cmd_restore_canvas(&client(&url), CHANNEL, REVISION, &mut vec![])
3822+
let mut out = vec![];
3823+
cmd_restore_canvas(&client(&url), CHANNEL, REVISION, &mut out)
38233824
.await
38243825
.expect("an accepted restore whose verification read fails still succeeds");
38253826
assert!(
38263827
submitted.lock().unwrap().is_some(),
38273828
"the restore did publish before the verification read failed"
38283829
);
3830+
let json: serde_json::Value = serde_json::from_slice(&out)
3831+
.expect("stdout must be valid JSON even when verification read fails");
3832+
assert!(
3833+
json.get("event_id").is_some(),
3834+
"stdout JSON must contain event_id"
3835+
);
3836+
assert!(
3837+
json.get("accepted").is_some(),
3838+
"stdout JSON must contain accepted"
3839+
);
3840+
assert!(
3841+
json.get("message").is_some(),
3842+
"stdout JSON must contain message"
3843+
);
38293844
}
38303845

38313846
/// `restore` sends `"consistency": "strong"` on the write-influencing
@@ -3888,8 +3903,8 @@ mod restore_canvas_tests {
38883903
/// seam in `submit_stored_event`:
38893904
///
38903905
/// 1. Attempt 1 — POST /events: A is persisted on the relay; the relay then
3891-
/// drops the TCP connection (no response body) so `with_retry_body` sees a
3892-
/// network/body error and retries with the same bytes.
3906+
/// returns HTTP 503 (a retryable status), so `with_retry_body` retries
3907+
/// with the same bytes.
38933908
/// 2. A concurrent writer B (built on A) arrives and becomes the new head
38943909
/// with `expected-revision = A`.
38953910
/// 3. Attempt 2 — POST /events: returns the canonical 409 conflict body

desktop/src/features/channels/ui/CanvasRestorePendingGuard.test.mjs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
* the originating row, and `set_canvas` was called exactly once.
1313
* 3. **No spurious second dispatch**: attempting another restore while the
1414
* mutation is pending (before the rejection settles) does not fire a second
15-
* `set_canvas` call — the Restore button also carries `disabled` while pending.
15+
* `set_canvas` call — the row expand buttons also carry `disabled` while
16+
* pending, preventing `reset()` from being called mid-flight.
1617
*
1718
* Revert-causality: removing `disabled={restoreMutation.isPending}` from the
1819
* row button un-disables the row during the pending phase. In that case the
@@ -65,7 +66,7 @@ let CommunitiesProvider;
6566
let CanvasHistoryPanel;
6667

6768
// Controls the IPC: null means the test has not triggered set_canvas yet.
68-
let deferredResolve = null;
69+
let _deferredResolve = null;
6970
let deferredReject = null;
7071
let setCanvasCalls = 0;
7172

@@ -101,7 +102,7 @@ before(async () => {
101102
if (cmd === "set_canvas") {
102103
setCanvasCalls += 1;
103104
return new Promise((resolve, reject) => {
104-
deferredResolve = resolve;
105+
_deferredResolve = resolve;
105106
deferredReject = reject;
106107
});
107108
}
@@ -162,15 +163,15 @@ async function settle(iterations = 8) {
162163
/**
163164
* Full pending-guard contract:
164165
*
165-
* 1. Row buttons are disabled while restoreMutation.isPending is true.
166+
* 1. Row expand buttons are disabled while restoreMutation.isPending is true.
166167
* 2. Rejecting the IPC makes the error visible under the originating row;
167168
* set_canvas remains at exactly 1 call (no reset() fired mid-flight).
168-
* 3. Clicking the Restore button while pending does not fire a second IPC
169-
* (the Restore button also carries disabled={restoreMutation.isPending}).
169+
* 3. Clicking another row's expand button while pending does not fire a second
170+
* IPC (disabled prevents reset() from being called mid-flight).
170171
*/
171172
test("pending-guard: row disabled, rejection visible, no second dispatch", async () => {
172173
setCanvasCalls = 0;
173-
deferredResolve = null;
174+
_deferredResolve = null;
174175
deferredReject = null;
175176

176177
const client = new QueryClient({
@@ -254,8 +255,10 @@ test("pending-guard: row disabled, rejection visible, no second dispatch", async
254255
});
255256
await settle(12);
256257

257-
// After rejection the error must render under the OLDER_A row.
258-
const errorEl = container.querySelector("[role='alert']");
258+
// After rejection the error must render under the OLDER_A row (the
259+
// originating row — OLDER_A was selected when set_canvas was dispatched).
260+
const olderALi = items[1]?.closest("li");
261+
const errorEl = olderALi?.querySelector("[role='alert']") ?? null;
259262
const errorVisible = errorEl !== null;
260263
const errorText = errorEl?.textContent ?? "";
261264

@@ -284,6 +287,10 @@ test("pending-guard: row disabled, rejection visible, no second dispatch", async
284287
errorVisible,
285288
"rejecting the IPC must render an error under the originating row",
286289
);
290+
assert.ok(
291+
errorText.includes(rejectionMessage),
292+
`error text must contain the rejection message; got: "${errorText}"`,
293+
);
287294
assert.equal(
288295
finalCallCount,
289296
1,

0 commit comments

Comments
 (0)