From 2c2bb9bba59e77c15e2f9e9d706344ec343772fa Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Sun, 11 Oct 2026 01:56:50 +0100 Subject: [PATCH] ci: run tests on Linux, Windows and macOS; add non-blocking lint job Tests previously ran on Ubuntu only, so Windows- and macOS-specific breakage went unseen. Add a 3-OS test matrix and a separate lint job for rustfmt and clippy. Lint is continue-on-error because the tree is not yet fmt-clean or clippy-clean. Also add BEDIT characterization tests that pin current session, undo and refusal behavior ahead of the editor refactor. Locked tests guard behavior that must survive; documenting tests record questionable behavior expected to change. --- .github/workflows/ci.yml | 50 ++++- src/app/command_driver/tests.rs | 336 ++++++++++++++++++++++++++++++++ 2 files changed, 378 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55066aa72..f4bfd33b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,8 @@ name: Tests -# Until now nothing ran the test suite on a change: the release workflows -# build the app, web-check.yml type-checks the wasm target, and pull requests -# reported their own local runs. Run the workspace tests on every push to -# main and on every pull request. There is deliberately no formatter check: -# the tree is not rustfmt-clean and is not going to be reformatted. +# Runs the workspace tests on every push to main and on every pull request, +# on Linux, Windows and macOS. A separate lint job reports rustfmt and clippy +# results without blocking merges until the existing tree has been cleaned up. on: push: @@ -20,13 +18,19 @@ concurrency: jobs: test: - runs-on: ubuntu-22.04 + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-22.04, windows-latest, macos-14] steps: - uses: actions/checkout@v4 # The same libraries the Linux release build needs (windowing, GL, # fonts); the packaging tools are left out. - - name: System libraries + - name: System libraries (Linux) + if: runner.os == 'Linux' run: | sudo apt-get update sudo apt-get install -y \ @@ -37,8 +41,38 @@ jobs: - uses: Swatinem/rust-cache@v2 - # LC_ALL=C: a few tests compare English message text. + # LC_ALL=C: a few tests compare English message text. The GPU-backed + # tests are #[ignore]d and need an adapter, so they do not run here. - name: cargo test env: LC_ALL: C run: cargo test --workspace --locked + + lint: + name: lint (informational) + runs-on: ubuntu-22.04 + # Non-blocking until the existing tree is cleaned up. The tree currently has + # thousands of rustfmt diffs and several clippy warnings and errors, so + # these checks report status without failing the workflow. + continue-on-error: true + steps: + - uses: actions/checkout@v4 + + - name: System libraries + run: | + sudo apt-get update + sudo apt-get install -y \ + libgl1-mesa-dev libx11-dev libxcursor-dev libxi-dev libxrandr-dev \ + libxkbcommon-dev libwayland-dev libfontconfig1-dev libfreetype6-dev + + - uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + + - uses: Swatinem/rust-cache@v2 + + - name: cargo fmt --check + run: cargo fmt --all -- --check + + - name: cargo clippy + run: cargo clippy --workspace --all-targets --locked diff --git a/src/app/command_driver/tests.rs b/src/app/command_driver/tests.rs index 85e6e3c8b..76ba84095 100644 --- a/src/app/command_driver/tests.rs +++ b/src/app/command_driver/tests.rs @@ -1133,3 +1133,339 @@ mod dispatched_command_task_tests { ); } } + +/// PW-1 BEDIT characterization tests. +/// +/// Two groups: +/// - `locked_*`: behavior Phase 1 must preserve (save keeps edits, discard +/// restores the definition, refusals stay refused). +/// - `documenting_*`: today's questionable behavior, pinned so the refactor +/// cannot change it silently. Each one is expected to flip in Phase 1. +#[cfg(test)] +mod bedit_characterization_tests { + use super::super::*; + use codec::entities::{Circle, Insert, Line}; + use codec::types::Vector3; + use codec::EntityType; + + /// Fresh document with one block definition `BLK` (one line) and one + /// model-space INSERT referencing it. Returns the INSERT handle and the + /// definition's member handles. + fn setup_block_and_insert(app: &mut OpenCADStudio) -> (Handle, Vec) { + let _ = app.automation_op(r#"{"op":"new"}"#); + let tab = app.active_tab; + app.tabs[tab].scene.define_block_raw( + "BLK", + Vector3::ZERO, + vec![EntityType::Line(Line::from_points( + Vector3::ZERO, + Vector3::new(10.0, 0.0, 0.0), + ))], + ); + let insert = app.tabs[tab] + .scene + .document + .add_entity(EntityType::Insert(Insert::new( + "BLK", + Vector3::new(100.0, 0.0, 0.0), + ))) + .expect("insert"); + let members = block_members(app, "BLK"); + (insert, members) + } + + fn block_members(app: &OpenCADStudio, name: &str) -> Vec { + app.tabs[app.active_tab] + .scene + .document + .block_records + .get(name) + .map(|br| br.entity_handles.clone()) + .unwrap_or_default() + } + + /// Definition state as (handle, Debug form) in member order. The Debug + /// form stands in for PartialEq, which EntityType does not provide. + fn definition_state(app: &OpenCADStudio, name: &str) -> Vec<(Handle, String)> { + let tab = app.active_tab; + block_members(app, name) + .into_iter() + .map(|h| { + let e = app.tabs[tab].scene.document.get_entity(h); + (h, format!("{e:?}")) + }) + .collect() + } + + fn undo_depth(app: &OpenCADStudio) -> usize { + app.tabs[app.active_tab].history.undo_stack.len() + } + + fn begin_bedit(app: &mut OpenCADStudio, insert: Handle) { + let _ = app.dispatch_command(&format!("BEDIT_BEGIN:{}", insert.value())); + assert!( + app.tabs[app.active_tab].active_block_edit.is_some(), + "setup: BEDIT_BEGIN should open a block-edit session" + ); + } + + fn add_circle_in_bedit(app: &mut OpenCADStudio) -> Handle { + let tab = app.active_tab; + app.tabs[tab].scene.add_entity(EntityType::Circle(Circle::from_coords( + 0.0, 0.0, 0.0, 2.0, + ))) + } + + // ---- locked: must survive Phase 1 ---- + + /// Locked: save keeps the entity added inside the session. + #[test] + fn locked_save_keeps_entity_added_in_session() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + begin_bedit(&mut app, insert); + add_circle_in_bedit(&mut app); + let _ = app.dispatch_command("BEDIT_SAVE"); + + assert!(app.tabs[app.active_tab].active_block_edit.is_none()); + assert_eq!(block_members(&app, "BLK").len(), 2, "save keeps the new entity"); + } + + /// Locked: discard restores the definition to its entry state exactly + /// (same handles, same geometry, same order). + #[test] + fn locked_discard_restores_definition_to_entry_state() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let entry = definition_state(&app, "BLK"); + begin_bedit(&mut app, insert); + add_circle_in_bedit(&mut app); + let _ = app.dispatch_command("BEDIT_DISCARD"); + + assert!(app.tabs[app.active_tab].active_block_edit.is_none()); + assert_eq!(definition_state(&app, "BLK"), entry); + } + + /// Locked: refusals. A non-INSERT is rejected and no session opens. + #[test] + fn locked_bedit_refuses_non_insert_object() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + let tab = app.active_tab; + let line = app.tabs[tab] + .scene + .document + .add_entity(EntityType::Line(Line::from_points( + Vector3::ZERO, + Vector3::new(1.0, 0.0, 0.0), + ))) + .expect("line"); + let _ = app.dispatch_command(&format!("BEDIT_BEGIN:{}", line.value())); + assert!(app.tabs[tab].active_block_edit.is_none()); + assert!(app.tabs[tab].block_edits.is_empty()); + } + + /// Locked: refusal. The `BEDIT` picker command is rejected while a + /// REFEDIT session is active and no BEDIT session opens. + #[test] + fn locked_bedit_picker_refused_while_refedit_active() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let tab = app.active_tab; + let _ = app.dispatch_command(&format!("REFEDIT_BEGIN:{}", insert.value())); + assert!( + app.tabs[tab].refedit_session.is_some(), + "setup: REFEDIT_BEGIN should open a REFEDIT session" + ); + let _ = app.dispatch_command("BEDIT"); + assert!(app.tabs[tab].active_block_edit.is_none()); + assert!(app.tabs[tab].block_edits.is_empty()); + } + + /// Documenting: the REFEDIT guard lives only in the `BEDIT` picker arm. + /// `BEDIT_BEGIN:` (used by the palette "Edit" action) has no + /// REFEDIT check and opens a session while REFEDIT is active. Expected to + /// flip in Phase 1 once the guard moves into the begin path. + #[test] + fn documenting_bedit_begin_bypasses_refedit_guard() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let tab = app.active_tab; + let _ = app.dispatch_command(&format!("REFEDIT_BEGIN:{}", insert.value())); + assert!(app.tabs[tab].refedit_session.is_some()); + let _ = app.dispatch_command(&format!("BEDIT_BEGIN:{}", insert.value())); + assert!( + app.tabs[tab].active_block_edit.is_some(), + "documenting: BEDIT_BEGIN opens a session despite an active REFEDIT" + ); + } + + /// Locked: refusal. An xref definition cannot be edited. + #[test] + fn locked_bedit_refuses_xref_definition() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let tab = app.active_tab; + app.tabs[tab] + .scene + .document + .block_records + .get_mut("BLK") + .expect("block record") + .flags + .is_xref = true; + let _ = app.dispatch_command(&format!("BEDIT_BEGIN:{}", insert.value())); + assert!(app.tabs[tab].active_block_edit.is_none()); + assert!(app.tabs[tab].block_edits.is_empty()); + } + + // ---- documenting: today's behavior, expected to flip in Phase 1 ---- + + /// Documenting: modify an existing member, discard, then undo once. The + /// undo target is the BEDIT_BEGIN entry. Records whatever the stack does + /// with the stale before-image (ISS-02). The assertion pins the current + /// result; Phase 1 must justify any change. + #[test] + fn documenting_modify_discard_then_undo_probes_stale_before_image() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, members) = setup_block_and_insert(&mut app); + let line = members[0]; + let entry = definition_state(&app, "BLK"); + begin_bedit(&mut app, insert); + + let tab = app.active_tab; + // Record the undo step before the mutation so the recorder captures it. + app.push_undo_snapshot(tab, "modify inside BEDIT"); + app.tabs[tab] + .scene + .document + .get_entity_mut(line) + .map(|e| { + if let EntityType::Line(l) = e { + l.end = Vector3::new(99.0, 0.0, 0.0); + } + }) + .expect("line in definition"); + app.finish_pending_history(tab); + + let _ = app.dispatch_command("BEDIT_DISCARD"); + app.finish_pending_history(tab); + assert_eq!(definition_state(&app, "BLK"), entry, "discard restores entry"); + + app.undo_steps(1); + // Pinned: the state after one undo, observed on current code. + assert_eq!( + definition_state(&app, "BLK"), + entry, + "documenting: undo after discard restores the entry state (ISS-02 not reproduced here)" + ); + } + + /// Documenting: undo during a session crosses the entry boundary. A + /// single undo after an in-session edit may revert the BEDIT_BEGIN step. + #[test] + fn documenting_undo_inside_session_crosses_entry_boundary() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + begin_bedit(&mut app, insert); + let tab = app.active_tab; + app.push_undo_snapshot(tab, "add circle in BEDIT"); + add_circle_in_bedit(&mut app); + app.finish_pending_history(tab); + assert_eq!(block_members(&app, "BLK").len(), 2); + + app.undo_steps(1); + // Documenting: undo removes the circle from the entity store but leaves + // its handle in the block record, so the definition holds a dangling + // member. Expected to flip in Phase 1 (members should match the store). + let members = block_members(&app, "BLK"); + assert_eq!(members.len(), 2, "documenting: block record keeps the undone handle"); + let dangling = members + .iter() + .filter(|h| app.tabs[tab].scene.document.get_entity(**h).is_none()) + .count(); + assert_eq!(dangling, 1, "documenting: one member handle has no entity"); + assert!( + app.tabs[tab].active_block_edit.is_some(), + "documenting: undo does not close the session" + ); + } + + /// Documenting: undoing the BEDIT_BEGIN step itself, while the session is + /// still open, leaves the editor open. Records whether the undo reaches + /// across the entry boundary. + #[test] + fn documenting_undo_across_entry_leaves_session_open() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let tab = app.active_tab; + begin_bedit(&mut app, insert); + app.finish_pending_history(tab); + let depth_after_begin = undo_depth(&app); + assert!(depth_after_begin >= 1, "BEDIT_BEGIN recorded an undo step"); + + app.undo_steps(1); + assert!( + app.tabs[tab].active_block_edit.is_some(), + "documenting: undoing BEDIT_BEGIN does not close the open session" + ); + } + + /// Documenting: BEDIT on a block while another block is open nests a + /// second session. Records the resulting stack. + #[test] + fn documenting_nested_bedit_opens_second_session() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + let tab = app.active_tab; + app.tabs[tab].scene.define_block_raw( + "OTHER", + Vector3::ZERO, + vec![EntityType::Line(Line::from_points( + Vector3::ZERO, + Vector3::new(5.0, 0.0, 0.0), + ))], + ); + let other_insert = app.tabs[tab] + .scene + .document + .add_entity(EntityType::Insert(Insert::new( + "OTHER", + Vector3::new(200.0, 0.0, 0.0), + ))) + .expect("insert"); + begin_bedit(&mut app, insert); + begin_bedit(&mut app, other_insert); + assert_eq!( + app.tabs[tab].block_edits.len(), + 2, + "documenting: a second BEDIT pushes a nested session" + ); + } + + /// Documenting: BEDIT_BEGIN pushes an undo entry, and BEDIT_SAVE and + /// BEDIT_DISCARD push none. Phase 1 may change this. + #[test] + fn documenting_save_and_discard_push_no_undo_entry() { + let mut app = OpenCADStudio::new_for_test(); + let (insert, _) = setup_block_and_insert(&mut app); + begin_bedit(&mut app, insert); + app.finish_pending_history(app.active_tab); + let after_begin = undo_depth(&app); + + let _ = app.dispatch_command("BEDIT_SAVE"); + app.finish_pending_history(app.active_tab); + assert_eq!(undo_depth(&app), after_begin, "save pushes no undo entry"); + + begin_bedit(&mut app, insert); + app.finish_pending_history(app.active_tab); + let before_discard = undo_depth(&app); + let _ = app.dispatch_command("BEDIT_DISCARD"); + app.finish_pending_history(app.active_tab); + assert_eq!( + undo_depth(&app), + before_discard, + "discard pushes no undo entry (not undoable)" + ); + } +}