From 6d386a884ed9d6d037eb52a85944ed1d6ff16982 Mon Sep 17 00:00:00 2001 From: Skyler Lehmkuhl Date: Wed, 17 Jun 2026 19:00:31 -0400 Subject: [PATCH] Phase 5: fix broken unit tests; cargo test --lib green again The lib unit tests had gone stale (time values became newtypes) and no longer compiled. Updated the test code to the current API and fixed the few real issues the now-running tests surfaced. Test-only: - Wrap raw f64 time literals in Beats(...) where the API now takes Beats (automation.rs); pass &TempoMap / Beats where signatures changed (clip.rs, effect_layer.rs). - shape.rs: assert the documented no-fill default (fill_color None) instead of Some. - add_clip_instance / trim_clip_instances tests: register a vector clip with the test's clip_id so the action's get_clip_duration lookup succeeds. Production fix (delete_folder.rs): - DeleteFolderAction(MoveToParent) reparented child subfolders to the deleted folder's parent but never restored them on undo, orphaning them. Track the moved subfolder ids and restore their parent on rollback. Result: daw-backend lib 17 passed; lightningbeam-core lib 264 passed. --- daw-backend/src/audio/automation.rs | 58 +++++++++---------- .../src/actions/add_clip_instance.rs | 5 +- .../src/actions/delete_folder.rs | 13 +++++ .../src/actions/trim_clip_instances.rs | 4 ++ .../lightningbeam-core/src/clip.rs | 11 +++- .../lightningbeam-core/src/effect_layer.rs | 10 +++- .../lightningbeam-core/src/shape.rs | 2 +- 7 files changed, 66 insertions(+), 37 deletions(-) diff --git a/daw-backend/src/audio/automation.rs b/daw-backend/src/audio/automation.rs index e592905..484c105 100644 --- a/daw-backend/src/audio/automation.rs +++ b/daw-backend/src/audio/automation.rs @@ -189,22 +189,22 @@ mod tests { fn test_add_points_sorted() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(2.0, 0.5, CurveType::Linear)); - lane.add_point(AutomationPoint::new(1.0, 0.3, CurveType::Linear)); - lane.add_point(AutomationPoint::new(3.0, 0.8, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(2.0), 0.5, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 0.3, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(3.0), 0.8, CurveType::Linear)); assert_eq!(lane.points().len(), 3); - assert_eq!(lane.points()[0].time, 1.0); - assert_eq!(lane.points()[1].time, 2.0); - assert_eq!(lane.points()[2].time, 3.0); + assert_eq!(lane.points()[0].time, Beats(1.0)); + assert_eq!(lane.points()[1].time, Beats(2.0)); + assert_eq!(lane.points()[2].time, Beats(3.0)); } #[test] fn test_replace_point_at_same_time() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(1.0, 0.3, CurveType::Linear)); - lane.add_point(AutomationPoint::new(1.0, 0.5, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 0.3, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 0.5, CurveType::Linear)); assert_eq!(lane.points().len(), 1); assert_eq!(lane.points()[0].value, 0.5); @@ -214,59 +214,59 @@ mod tests { fn test_linear_interpolation() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(0.0, 0.0, CurveType::Linear)); - lane.add_point(AutomationPoint::new(1.0, 1.0, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(0.0), 0.0, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 1.0, CurveType::Linear)); - assert_eq!(lane.evaluate(0.0), Some(0.0)); - assert_eq!(lane.evaluate(0.5), Some(0.5)); - assert_eq!(lane.evaluate(1.0), Some(1.0)); + assert_eq!(lane.evaluate(Beats(0.0)), Some(0.0)); + assert_eq!(lane.evaluate(Beats(0.5)), Some(0.5)); + assert_eq!(lane.evaluate(Beats(1.0)), Some(1.0)); } #[test] fn test_step_interpolation() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(0.0, 0.5, CurveType::Step)); - lane.add_point(AutomationPoint::new(1.0, 1.0, CurveType::Step)); + lane.add_point(AutomationPoint::new(Beats(0.0), 0.5, CurveType::Step)); + lane.add_point(AutomationPoint::new(Beats(1.0), 1.0, CurveType::Step)); - assert_eq!(lane.evaluate(0.0), Some(0.5)); - assert_eq!(lane.evaluate(0.5), Some(0.5)); - assert_eq!(lane.evaluate(0.99), Some(0.5)); - assert_eq!(lane.evaluate(1.0), Some(1.0)); + assert_eq!(lane.evaluate(Beats(0.0)), Some(0.5)); + assert_eq!(lane.evaluate(Beats(0.5)), Some(0.5)); + assert_eq!(lane.evaluate(Beats(0.99)), Some(0.5)); + assert_eq!(lane.evaluate(Beats(1.0)), Some(1.0)); } #[test] fn test_evaluate_outside_range() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(1.0, 0.5, CurveType::Linear)); - lane.add_point(AutomationPoint::new(2.0, 1.0, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 0.5, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(2.0), 1.0, CurveType::Linear)); // Before first point - assert_eq!(lane.evaluate(0.0), Some(0.5)); + assert_eq!(lane.evaluate(Beats(0.0)), Some(0.5)); // After last point - assert_eq!(lane.evaluate(3.0), Some(1.0)); + assert_eq!(lane.evaluate(Beats(3.0)), Some(1.0)); } #[test] fn test_disabled_lane() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(0.0, 0.5, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(0.0), 0.5, CurveType::Linear)); lane.enabled = false; - assert_eq!(lane.evaluate(0.0), None); + assert_eq!(lane.evaluate(Beats(0.0)), None); } #[test] fn test_remove_point() { let mut lane = AutomationLane::new(0, ParameterId::TrackVolume); - lane.add_point(AutomationPoint::new(1.0, 0.5, CurveType::Linear)); - lane.add_point(AutomationPoint::new(2.0, 0.8, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(1.0), 0.5, CurveType::Linear)); + lane.add_point(AutomationPoint::new(Beats(2.0), 0.8, CurveType::Linear)); - assert!(lane.remove_point_at_time(1.0, 0.001)); + assert!(lane.remove_point_at_time(Beats(1.0), Beats(0.001))); assert_eq!(lane.points().len(), 1); - assert_eq!(lane.points()[0].time, 2.0); + assert_eq!(lane.points()[0].time, Beats(2.0)); } } diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/add_clip_instance.rs b/lightningbeam-ui/lightningbeam-core/src/actions/add_clip_instance.rs index 86fb996..a1f438b 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/add_clip_instance.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/add_clip_instance.rs @@ -316,8 +316,11 @@ mod tests { let layer_id = layer.layer.id; document.root_mut().add_child(AnyLayer::Vector(layer)); - // Create a clip instance (using a fake clip_id since we're just testing the action) + // Register a vector clip so get_clip_duration succeeds let clip_id = Uuid::new_v4(); + let clip = crate::clip::VectorClip::with_id(clip_id, "Test Clip", 100.0, 100.0, 5.0); + document.vector_clips.insert(clip_id, clip); + let clip_instance = ClipInstance::new(clip_id); let instance_id = clip_instance.id; diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/delete_folder.rs b/lightningbeam-ui/lightningbeam-core/src/actions/delete_folder.rs index 83d647c..2681265 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/delete_folder.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/delete_folder.rs @@ -37,6 +37,9 @@ pub struct DeleteFolderAction { /// Asset IDs that were deleted (for DeleteRecursive strategy) deleted_asset_ids: Vec, + + /// Subfolder IDs that were reparented to the deleted folder's parent (for MoveToParent strategy) + moved_subfolder_ids: Vec, } impl DeleteFolderAction { @@ -55,6 +58,7 @@ impl DeleteFolderAction { removed_folders: Vec::new(), moved_asset_ids: Vec::new(), deleted_asset_ids: Vec::new(), + moved_subfolder_ids: Vec::new(), } } } @@ -130,6 +134,7 @@ impl Action for DeleteFolderAction { for subfolder_id in subfolder_ids { if let Some(subfolder) = tree.folders.get_mut(&subfolder_id) { subfolder.parent_id = parent_id; + self.moved_subfolder_ids.push(subfolder_id); } } } @@ -259,6 +264,13 @@ impl Action for DeleteFolderAction { tree.add_folder(folder.clone()); } + // Restore reparented subfolders back under the deleted folder + for subfolder_id in &self.moved_subfolder_ids { + if let Some(subfolder) = tree.folders.get_mut(subfolder_id) { + subfolder.parent_id = Some(self.folder_id); + } + } + match self.strategy { DeleteStrategy::MoveToParent => { // Restore folder_id for moved assets @@ -312,6 +324,7 @@ impl Action for DeleteFolderAction { self.removed_folders.clear(); self.moved_asset_ids.clear(); self.deleted_asset_ids.clear(); + self.moved_subfolder_ids.clear(); Ok(()) } diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs b/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs index ee349b8..9595ba3 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs @@ -547,6 +547,8 @@ mod tests { // Create a clip ID (ClipInstance references clip by ID) let clip_id = uuid::Uuid::new_v4(); + let clip = crate::clip::VectorClip::with_id(clip_id, "Test Clip", 100.0, 100.0, 10.0); + document.vector_clips.insert(clip_id, clip); let mut vector_layer = VectorLayer::new("Layer 1"); @@ -607,6 +609,8 @@ mod tests { // Create a clip ID (ClipInstance references clip by ID) let clip_id = uuid::Uuid::new_v4(); + let clip = crate::clip::VectorClip::with_id(clip_id, "Test Clip", 100.0, 100.0, 10.0); + document.vector_clips.insert(clip_id, clip); let mut vector_layer = VectorLayer::new("Layer 1"); diff --git a/lightningbeam-ui/lightningbeam-core/src/clip.rs b/lightningbeam-ui/lightningbeam-core/src/clip.rs index 68d6fb0..6367c9b 100644 --- a/lightningbeam-ui/lightningbeam-core/src/clip.rs +++ b/lightningbeam-ui/lightningbeam-core/src/clip.rs @@ -902,7 +902,7 @@ mod tests { #[test] fn test_audio_clip_midi() { - let clip = AudioClip::new_midi("Piano Melody", 1, 60.0); + let clip = AudioClip::new_midi("Piano Melody", 1, daw_backend::Beats(60.0)); assert_eq!(clip.name, "Piano Melody"); assert_eq!(clip.duration, 60.0); match &clip.clip_type { @@ -952,7 +952,10 @@ mod tests { assert_eq!(instance.trim_start, 2.0); assert_eq!(instance.trim_end, Some(8.0)); - assert_eq!(instance.effective_duration(10.0), 6.0); + // At 60 BPM the tempo map is identity (1 beat == 1 second), so the + // beats-domain effective duration equals the seconds content window. + let tempo_map = crate::tempo_map::TempoMap::constant(60.0); + assert_eq!(instance.effective_duration(10.0, &tempo_map), 6.0); } #[test] @@ -963,7 +966,9 @@ mod tests { assert_eq!(instance.trim_start, 2.0); assert_eq!(instance.trim_end, None); - assert_eq!(instance.effective_duration(10.0), 8.0); + // At 60 BPM the tempo map is identity (1 beat == 1 second). + let tempo_map = crate::tempo_map::TempoMap::constant(60.0); + assert_eq!(instance.effective_duration(10.0, &tempo_map), 8.0); } #[test] diff --git a/lightningbeam-ui/lightningbeam-core/src/effect_layer.rs b/lightningbeam-ui/lightningbeam-core/src/effect_layer.rs index fb919e8..3a830ba 100644 --- a/lightningbeam-ui/lightningbeam-core/src/effect_layer.rs +++ b/lightningbeam-ui/lightningbeam-core/src/effect_layer.rs @@ -240,14 +240,18 @@ mod tests { let effect2 = def.create_instance(3.0, 7.0); // 3.0 + 7.0 = 10.0 end layer.add_clip_instance(effect2); + // At 60 BPM the tempo map is identity (1 beat == 1 second), so the + // query time in seconds equals the effect's beats-domain extents. + let tempo_map = crate::tempo_map::TempoMap::constant(60.0); + // At time 2: only effect1 active - assert_eq!(layer.active_clip_instances_at(2.0, 60.0).len(), 1); + assert_eq!(layer.active_clip_instances_at(2.0, &tempo_map).len(), 1); // At time 4: both effects active - assert_eq!(layer.active_clip_instances_at(4.0, 60.0).len(), 2); + assert_eq!(layer.active_clip_instances_at(4.0, &tempo_map).len(), 2); // At time 7: only effect2 active - assert_eq!(layer.active_clip_instances_at(7.0, 60.0).len(), 1); + assert_eq!(layer.active_clip_instances_at(7.0, &tempo_map).len(), 1); } #[test] diff --git a/lightningbeam-ui/lightningbeam-core/src/shape.rs b/lightningbeam-ui/lightningbeam-core/src/shape.rs index 84d7851..cd08af6 100644 --- a/lightningbeam-ui/lightningbeam-core/src/shape.rs +++ b/lightningbeam-ui/lightningbeam-core/src/shape.rs @@ -395,7 +395,7 @@ mod tests { let shape = Shape::new(path); assert_eq!(shape.versions.len(), 1); - assert!(shape.fill_color.is_some()); + assert!(shape.fill_color.is_none()); } #[test]