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.
This commit is contained in:
Skyler Lehmkuhl 2026-06-17 19:00:31 -04:00
parent 4ad95e6755
commit 6d386a884e
7 changed files with 66 additions and 37 deletions

View File

@ -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));
}
}

View File

@ -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;

View File

@ -37,6 +37,9 @@ pub struct DeleteFolderAction {
/// Asset IDs that were deleted (for DeleteRecursive strategy)
deleted_asset_ids: Vec<Uuid>,
/// Subfolder IDs that were reparented to the deleted folder's parent (for MoveToParent strategy)
moved_subfolder_ids: Vec<Uuid>,
}
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(())
}

View File

@ -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");

View File

@ -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]

View File

@ -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]

View File

@ -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]