Files
jellytau/src-tauri/src/player/mpv_backend_test.rs
T
dtourolle 8500da1a42 chore(rust): clear the clippy backlog and finish the poison-tolerant lock sweep
`cargo clippy --all-targets` went from 51 warnings (23 in the lib) to zero.
Most were mechanical — needless borrows, `assert_eq!` against a bool literal,
`vec!` where an array does, `or_insert_with(Vec::new)`, a loop index used only
to index — and were applied with `clippy --fix`, then reviewed line by line.
That review caught one auto-fix that was *not* semantically neutral: dropping
the redundant `use hostname;` left its `#[cfg(target_os = "linux")]` orphaned
directly above `SERVICE_NAME`, which would have silently cfg'd the constant out
of every non-Linux build. Removed the stray attribute with the import.

Where a lint asked for a risky change rather than a better one, it is suppressed
with a comment saying why:

- `too_many_arguments` on five `#[tauri::command]` handlers and
  `ThumbnailCache::save_thumbnail` — most of the arity is `State<'_, _>`
  injection, and a parameter struct would change the IPC contract and the
  generated TypeScript for no readability gain.
- `large_enum_variant` on `PlayerStatusEvent` and `AutoplayDecision` — both are
  serde + specta wire types emitted a handful of times a second, never bulk
  allocated; boxing would have to stay invisible to the generated bindings while
  every match arm gained a deref.
- `await_holding_lock` on the `hybrid`/`offline` test modules — the guard is a
  test-only serialisation lock for the process-global `INCLUDE_CATALOG_BROWSE`
  flag, and the await it spans *is* the critical section. Each `#[tokio::test]`
  gets its own single-threaded runtime, so this is not the production deadlock
  class the lint targets; restructuring would reintroduce the flag race.

Real fixes elsewhere: `JellyfinItem::to_media_item` takes `self` by value, so it
is now `into_media_item`; the five-tuple episode row in the download commands
has a named `EpisodeRow` alias; the mpv `PropertyChange` arm matches
`name: "pause"` instead of guarding on it.

Also converted the last 27 raw `.lock().unwrap()` call sites to `lock_safe()`,
completing the `MutexSafe`/`RwLockSafe` convention. All of them turned out to be
in test modules — production code was already clean — so this is consistency
rather than a fix. The two raw locks in `utils/lock.rs` stay raw on purpose:
those tests deliberately poison a mutex to prove the helpers recover from it.

Pure refactoring: all 698 tests still pass.
2026-08-16 23:05:13 +02:00

352 lines
12 KiB
Rust

//! Tests for MpvBackend to prevent regressions
//!
//! These tests are designed to catch common issues like:
//! - Tokio runtime panics when spawning async tasks from std::thread
//! - Position update thread failures
//! - Event emission issues
//!
//! TRACES: UR-003, UR-004 | IR-003 | IT-003, IT-004
#[cfg(test)]
mod tests {
use crate::utils::lock::MutexSafe;
use std::sync::{Arc, Mutex};
use tokio::sync::Mutex as TokioMutex;
/// Test that simulates the position update thread spawning async tasks
/// without a Tokio runtime (the bug we just fixed)
#[test]
fn test_position_thread_handles_missing_tokio_runtime() {
use std::sync::atomic::{AtomicBool, Ordering};
let success = Arc::new(AtomicBool::new(false));
let success_clone = success.clone();
// Spawn a regular thread (no Tokio runtime)
let handle = std::thread::spawn(move || {
// This simulates what the position update thread does
// It should handle the case where there's no Tokio runtime
// Try to get the current Tokio runtime handle
if let Ok(handle) = tokio::runtime::Handle::try_current() {
// We have a runtime, use it
handle.spawn(async move {
// Async work here
});
} else {
// No runtime, spawn a new thread with its own runtime
// This is the fix we applied
std::thread::spawn(move || {
let rt = tokio::runtime::Runtime::new().unwrap();
rt.block_on(async move {
// Async work here
success_clone.store(true, Ordering::SeqCst);
});
});
}
});
handle.join().unwrap();
// Give the spawned thread time to complete
std::thread::sleep(std::time::Duration::from_millis(100));
assert!(
success.load(Ordering::SeqCst),
"Should successfully execute async code from std::thread without panicking"
);
}
/// Test that the Tokio runtime fallback pattern works correctly
#[test]
fn test_tokio_runtime_fallback_pattern() {
let counter = Arc::new(Mutex::new(0));
let counter_clone = counter.clone();
// Spawn from a regular thread (no runtime)
let handle = std::thread::spawn(move || {
if let Ok(handle) = tokio::runtime::Handle::try_current() {
// Has runtime (shouldn't happen in this test)
handle.spawn(async move {
*counter_clone.lock_safe() += 1;
});
} else {
// No runtime - use fallback (should happen in this test)
std::thread::spawn(move || {
let rt = tokio::runtime::Runtime::new().unwrap();
rt.block_on(async move {
*counter_clone.lock_safe() += 1;
});
});
}
});
handle.join().unwrap();
// Wait for async task to complete
std::thread::sleep(std::time::Duration::from_millis(100));
let count = *counter.lock_safe();
assert_eq!(
count, 1,
"Fallback pattern should execute async code successfully"
);
}
/// Test that position update logic works in a thread
#[test]
fn test_position_update_in_thread() {
use std::time::Duration;
let positions = Arc::new(Mutex::new(Vec::new()));
let positions_clone = positions.clone();
// Simulate the position update thread
let handle = std::thread::spawn(move || {
for i in 0..5 {
std::thread::sleep(Duration::from_millis(10));
// Simulate getting position from player
let position = i as f64 * 0.25;
// Store position (simulating event emission)
positions_clone.lock_safe().push(position);
}
});
handle.join().unwrap();
let recorded_positions = positions.lock_safe();
assert_eq!(
recorded_positions.len(),
5,
"Should have recorded 5 position updates"
);
// Verify positions are increasing
for (i, pos) in recorded_positions.iter().enumerate() {
let expected = i as f64 * 0.25;
assert!(
(*pos - expected).abs() < 0.001,
"Position {} should be close to {}",
pos,
expected
);
}
}
/// Test async progress reporting pattern
#[tokio::test]
async fn test_progress_reporting_with_tokio_mutex() {
use crate::playback_reporting::{EventThrottler, PlaybackReporter};
// Create mock reporter (None for this test)
let reporter = Arc::new(TokioMutex::new(None::<PlaybackReporter>));
let throttler = Arc::new(EventThrottler::new());
// Simulate progress reporting
let item_id = "test_item_123".to_string();
// This should not panic even though reporter is None
let reporter_guard = reporter.lock().await;
if let Some(_reporter_instance) = reporter_guard.as_ref() {
// Would report here if reporter was configured
} else {
// Reporter not configured - this is OK
}
drop(reporter_guard);
// Verify throttler works
assert!(
throttler.should_report(&item_id),
"First report should be allowed"
);
throttler.mark_reported(&item_id);
// Immediate second report should be throttled
// (EventThrottler has internal logic for this)
}
/// Test that position updates are emitted even when paused (for scrubbing)
/// This is critical for UI responsiveness when seeking while paused
#[test]
fn test_position_updates_while_paused() {
use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::Duration;
let update_count = Arc::new(AtomicUsize::new(0));
let update_count_clone = update_count.clone();
// Simulate a position update thread that runs regardless of pause state
let handle = std::thread::spawn(move || {
// Simulate 5 position updates
for _ in 0..5 {
std::thread::sleep(Duration::from_millis(50));
// In the real implementation, we check position from MPV
// and emit PositionUpdate events even when paused
// This simulates that behavior:
let _is_paused = true; // Simulating paused state
// Key: We DON'T skip the update when paused
// This allows scrubbing to work
update_count_clone.fetch_add(1, Ordering::SeqCst);
}
});
handle.join().unwrap();
let final_count = update_count.load(Ordering::SeqCst);
assert_eq!(
final_count, 5,
"Position updates should be emitted even when paused (got {} updates)",
final_count
);
}
/// Test that progress reporting is skipped when paused
/// Progress reporting to the server should only happen during active playback
#[test]
fn test_progress_reporting_skipped_when_paused() {
use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::Duration;
let report_count = Arc::new(AtomicUsize::new(0));
let report_count_clone = report_count.clone();
// Simulate the progress reporting logic
let handle = std::thread::spawn(move || {
// Simulate 5 update cycles
for i in 0..5 {
std::thread::sleep(Duration::from_millis(50));
// Position updates are emitted (tested separately)
// But progress reporting depends on pause state
let is_paused = i % 2 == 0; // Alternate between paused and playing
// Key: Only report when NOT paused
if !is_paused {
report_count_clone.fetch_add(1, Ordering::SeqCst);
}
}
});
handle.join().unwrap();
let final_count = report_count.load(Ordering::SeqCst);
assert_eq!(
final_count, 2,
"Progress reporting should only happen when not paused (got {} reports)",
final_count
);
}
/// Test that position updates are suppressed briefly after a seek
/// This prevents "jumping to zero" visual glitches during seek operations
#[test]
fn test_position_updates_suppressed_after_seek() {
use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
let last_seek_time = Arc::new(AtomicU64::new(0));
let last_seek_time_clone = last_seek_time.clone();
let update_count = Arc::new(AtomicUsize::new(0));
let update_count_clone = update_count.clone();
// Simulate a seek happening
let seek_time = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_millis() as u64;
last_seek_time.store(seek_time, Ordering::Relaxed);
// Simulate position update thread
let handle = std::thread::spawn(move || {
// Try 2 position updates at 50ms intervals (well within the 150ms window)
for _ in 0..2 {
std::thread::sleep(Duration::from_millis(50));
// Check if we should suppress updates (within 150ms of seek)
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_millis() as u64;
let last_seek = last_seek_time_clone.load(Ordering::Relaxed);
let time_since_seek = now.saturating_sub(last_seek);
if time_since_seek < 150 {
// Suppress update (don't increment counter)
continue;
}
// Emit update
update_count_clone.fetch_add(1, Ordering::SeqCst);
}
});
handle.join().unwrap();
let final_count = update_count.load(Ordering::SeqCst);
// With 50ms intervals and 150ms suppression window, updates at 50ms and 100ms
// should both be suppressed
assert_eq!(
final_count, 0,
"Position updates should be suppressed within 150ms of seek (got {} updates)",
final_count
);
}
/// Test that position updates resume after seek suppression window
#[test]
fn test_position_updates_resume_after_seek_window() {
use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
let last_seek_time = Arc::new(AtomicU64::new(0));
let last_seek_time_clone = last_seek_time.clone();
let update_count = Arc::new(AtomicUsize::new(0));
let update_count_clone = update_count.clone();
// Simulate a seek that happened 200ms ago (past the suppression window)
let seek_time = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_millis() as u64
- 200; // 200ms ago
last_seek_time.store(seek_time, Ordering::Relaxed);
// Simulate position update thread
let handle = std::thread::spawn(move || {
// Try 3 position updates
for _ in 0..3 {
std::thread::sleep(Duration::from_millis(10));
// Check if we should suppress updates (within 150ms of seek)
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_millis() as u64;
let last_seek = last_seek_time_clone.load(Ordering::Relaxed);
let time_since_seek = now.saturating_sub(last_seek);
if time_since_seek < 150 {
continue; // Should not happen in this test
}
// Emit update
update_count_clone.fetch_add(1, Ordering::SeqCst);
}
});
handle.join().unwrap();
let final_count = update_count.load(Ordering::SeqCst);
assert_eq!(
final_count, 3,
"Position updates should resume after seek suppression window (got {} updates)",
final_count
);
}
}