Skip to content

Commit e763034

Browse files
claudesinelaw
authored andcommitted
flash tests: comply with CONTRIBUTING rule #2 (observe, not inspect)
Existing flash e2e tests peeked at editor model state — `editor_mode()`, `cursor_position()`, `command_registry()` — to wait/assert. CONTRIBUTING is explicit: "asserts only on rendered output." vi_mode tests follow the same anti-pattern, but new code should be cleaner. Why it matters here: the model-state accessors and the rendered screen aren't always in lockstep under load. `wait_until` ticks every 50ms and may see `editor_mode == "flash"` set in the snapshot before the corresponding `setStatus("Flash[]")` command has been processed by the render side. The CI hangs (6 flash tests TIMEOUT at 180s under high parallelism) suggest the existing tests were sensitive to that timing — passes locally where contention is lower. Changes: - arm_flash: drops the command_registry.get_all() peek and the `editor_mode()` peek. Single readiness signal: the visible `Flash[]` status banner. This is the same signal the plugin's setStatus call sets *inside* the main loop, so seeing it on screen proves the editor has processed setEditorMode AND beginKeyCapture AND the first iteration's setStatus — guarantees the next getNextKey is armed. - All `wait_until(editor_mode != "flash")` waits → screen-only `wait_until(!screen.contains("Flash["))`. - All `cursor_position()` assertions → marker-glyph assertions: send a unique character (`@`) after the jump and check the rendered buffer for `@hello there` etc. Final flash tests now use exclusively screen-observable predicates. All 6 flash + 46 vi_mode + 41 markdown_compose tests still pass locally. Hoping this also unblocks CI by removing the model/render race window.
1 parent cb30e50 commit e763034

1 file changed

Lines changed: 75 additions & 75 deletions

File tree

crates/fresh-editor/tests/e2e/flash.rs

Lines changed: 75 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::common::fixtures::TestFixture;
1212
use crate::common::harness::{copy_plugin, copy_plugin_lib, EditorTestHarness};
1313
use crate::common::tracing::init_tracing_from_env;
1414
use crossterm::event::{KeyCode, KeyModifiers};
15-
use fresh::input::keybindings::Action::PluginAction;
1615
use std::fs;
1716

1817
/// Build a harness with the `flash` plugin loaded into an isolated
@@ -39,19 +38,12 @@ fn flash_harness(width: u16, height: u16) -> (EditorTestHarness, tempfile::TempD
3938
}
4039

4140
/// Open the command palette, type `Flash: Jump`, press Enter, and
42-
/// wait for the plugin's mode + status to be set. Mirrors
43-
/// `enable_vi_mode` in vi_mode tests.
41+
/// wait for the plugin's status banner (`Flash[]`) to appear on
42+
/// screen. CONTRIBUTING rule #2 — observe only rendered output —
43+
/// so we don't peek at `editor_mode()` or the command registry;
44+
/// the visible `Flash[]` banner is the single signal that the
45+
/// plugin is in flash mode AND has armed its first `getNextKey`.
4446
fn arm_flash(harness: &mut EditorTestHarness) {
45-
// Wait for the plugin's command to be registered.
46-
harness
47-
.wait_until(|h| {
48-
let commands = h.editor().command_registry().read().unwrap().get_all();
49-
commands
50-
.iter()
51-
.any(|c| c.action == PluginAction("flash_jump".to_string()))
52-
})
53-
.unwrap();
54-
5547
harness
5648
.send_key(KeyCode::Char('p'), KeyModifiers::CONTROL)
5749
.unwrap();
@@ -63,15 +55,12 @@ fn arm_flash(harness: &mut EditorTestHarness) {
6355
.send_key(KeyCode::Enter, KeyModifiers::NONE)
6456
.unwrap();
6557

66-
// Wait for the plugin to enter flash mode AND post its initial
67-
// empty-pattern status. Both signals together prove the plugin
68-
// has armed its first `getNextKey` and is ready for the next key.
69-
harness
70-
.wait_until(|h| {
71-
h.editor().editor_mode() == Some("flash".to_string())
72-
&& h.screen_to_string().contains("Flash[]")
73-
})
74-
.unwrap();
58+
// The empty-pattern status banner is the readiness signal. It
59+
// is set inside the plugin's main loop AFTER `setEditorMode` and
60+
// `beginKeyCapture` have been queued, so seeing it on screen
61+
// proves the editor has processed all three. No model peek
62+
// required.
63+
harness.wait_for_screen_contains("Flash[]").unwrap();
7564
}
7665

7766
/// Type a pattern one character at a time, waiting after each char
@@ -96,35 +85,40 @@ fn type_pattern(harness: &mut EditorTestHarness, pattern: &str) {
9685

9786
#[test]
9887
fn flash_jumps_to_label() {
99-
// Three "hello" lines; cursor at byte 0. Distances 0/12/24
100-
// → labels a/s/d in distance order. Pressing 's' jumps to
101-
// byte 12 (start of "hello there").
88+
// Three "hello" lines. After arming flash and typing pattern
89+
// "hello", labels are assigned in distance order from the
90+
// cursor (currently at line 1 col 1). We press 's' (the second
91+
// pool letter after the labeler's skip rule) to jump. To keep
92+
// the assertion screen-only (CONTRIBUTING rule #2), we then
93+
// insert a marker character and observe where it lands in the
94+
// rendered buffer.
10295
let (mut harness, _temp) = flash_harness(120, 24);
10396
let fixture = TestFixture::new("test.txt", "hello world\nhello there\nhello again\n").unwrap();
10497
harness.open_file(&fixture.path).unwrap();
10598
harness.render().unwrap();
10699

107-
let initial = harness.cursor_position();
108-
109100
arm_flash(&mut harness);
110101
type_pattern(&mut harness, "hello");
111102
harness.render().unwrap();
112103

113104
harness
114105
.send_key(KeyCode::Char('s'), KeyModifiers::NONE)
115106
.unwrap();
116-
117107
harness
118-
.wait_until(|h| h.editor().editor_mode() != Some("flash".to_string()))
108+
.wait_until(|h| !h.screen_to_string().contains("Flash["))
119109
.unwrap();
120110

121-
let landed = harness.cursor_position();
122-
assert_ne!(landed, initial, "cursor should have moved");
123-
assert_eq!(
124-
landed, 12,
125-
"expected cursor at start of second match (byte 12), got {}",
126-
landed,
127-
);
111+
// Insert a marker so we can observe where the cursor landed.
112+
harness
113+
.send_key(KeyCode::Char('@'), KeyModifiers::NONE)
114+
.unwrap();
115+
harness
116+
.wait_until(|h| h.screen_to_string().contains("@hello there"))
117+
.unwrap();
118+
// Negative check: the `@` did NOT land on line 1 or line 3.
119+
let screen = harness.screen_to_string();
120+
assert!(!screen.contains("@hello world"), "screen:\n{}", screen);
121+
assert!(!screen.contains("@hello again"), "screen:\n{}", screen);
128122
}
129123

130124
#[test]
@@ -134,23 +128,23 @@ fn flash_escape_cancels_no_movement() {
134128
harness.open_file(&fixture.path).unwrap();
135129
harness.render().unwrap();
136130

137-
let initial = harness.cursor_position();
138-
139131
arm_flash(&mut harness);
140132
type_pattern(&mut harness, "hello");
141133
harness.render().unwrap();
142134

143135
harness.send_key(KeyCode::Esc, KeyModifiers::NONE).unwrap();
144-
145136
harness
146-
.wait_until(|h| h.editor().editor_mode() != Some("flash".to_string()))
137+
.wait_until(|h| !h.screen_to_string().contains("Flash["))
147138
.unwrap();
148139

149-
assert_eq!(
150-
harness.cursor_position(),
151-
initial,
152-
"Escape must not move the cursor",
153-
);
140+
// Cursor must still be at the start of line 1. Insert a marker
141+
// and observe the rendered text.
142+
harness
143+
.send_key(KeyCode::Char('@'), KeyModifiers::NONE)
144+
.unwrap();
145+
harness
146+
.wait_until(|h| h.screen_to_string().contains("@hello world"))
147+
.unwrap();
154148
}
155149

156150
#[test]
@@ -181,16 +175,17 @@ fn flash_backspace_shrinks_pattern() {
181175
harness
182176
.send_key(KeyCode::Char('d'), KeyModifiers::NONE)
183177
.unwrap();
184-
185178
harness
186-
.wait_until(|h| h.editor().editor_mode() != Some("flash".to_string()))
179+
.wait_until(|h| !h.screen_to_string().contains("Flash["))
187180
.unwrap();
188181

189-
assert_eq!(
190-
harness.cursor_position(),
191-
24,
192-
"after backspace+retype, label 'd' must reach line 3 (byte 24)",
193-
);
182+
// Marker assertion: cursor landed at start of line 3.
183+
harness
184+
.send_key(KeyCode::Char('@'), KeyModifiers::NONE)
185+
.unwrap();
186+
harness
187+
.wait_until(|h| h.screen_to_string().contains("@hello again"))
188+
.unwrap();
194189
}
195190

196191
/// Regression for the silent-conceal bug, 2026-04: flash relies on
@@ -321,33 +316,31 @@ fn flash_jumps_across_splits() {
321316
.unwrap();
322317

323318
harness
324-
.wait_until(|h| h.editor().editor_mode() != Some("flash".to_string()))
319+
.wait_until(|h| !h.screen_to_string().contains("Flash["))
325320
.unwrap();
326321

327-
// The left split should now be active, and its cursor should be at
328-
// byte 0 (start of "alpha left side").
329-
let active_buf = harness.editor().active_buffer();
330-
let cursor = harness.cursor_position();
331-
assert_eq!(
332-
cursor, 0,
333-
"expected cursor at byte 0 of left split's buffer, got {}",
334-
cursor,
335-
);
336-
// And the active buffer should be the LEFT one — verify by reading
337-
// its file path through the public buffer info on screen.
322+
// Insert a marker — it should land at the start of the LEFT
323+
// split's "alpha left side", i.e. on the same line as that text
324+
// in the left split. Screen-only assertion (CONTRIBUTING #2).
325+
harness
326+
.send_key(KeyCode::Char('@'), KeyModifiers::NONE)
327+
.unwrap();
328+
harness
329+
.wait_until(|h| h.screen_to_string().contains("@alpha left side"))
330+
.unwrap();
331+
// Negative check: the right split's `alpha right side` is still
332+
// there but unmarked.
338333
let screen = harness.screen_to_string();
339334
assert!(
340-
screen.contains("left.txt"),
341-
"left.txt should still be visible; screen:\n{}",
335+
screen.contains("alpha right side"),
336+
"right split's content should remain visible; screen:\n{}",
337+
screen,
338+
);
339+
assert!(
340+
!screen.contains("@alpha right side"),
341+
"marker should NOT have landed in the right split; screen:\n{}",
342342
screen,
343343
);
344-
// Defensive: ensure we didn't somehow stay in the right split.
345-
// (We don't have a single accessor for "active buffer path" in the
346-
// harness, but we can check the buffer id is not the right one's.
347-
// The simplest reliable cross-check is that the cursor moved; in a
348-
// single-split run it would still be at the original right-side
349-
// byte.)
350-
let _ = active_buf;
351344
}
352345

353346
#[test]
@@ -368,8 +361,15 @@ fn flash_enter_jumps_to_closest() {
368361
.unwrap();
369362

370363
harness
371-
.wait_until(|h| h.editor().editor_mode() != Some("flash".to_string()))
364+
.wait_until(|h| !h.screen_to_string().contains("Flash["))
372365
.unwrap();
373366

374-
assert_eq!(harness.cursor_position(), 0);
367+
// Marker assertion — should land before the very first char of
368+
// line 1. (CONTRIBUTING #2: screen-only.)
369+
harness
370+
.send_key(KeyCode::Char('@'), KeyModifiers::NONE)
371+
.unwrap();
372+
harness
373+
.wait_until(|h| h.screen_to_string().contains("@hello world"))
374+
.unwrap();
375375
}

0 commit comments

Comments
 (0)