Build & Release / Create Release (push) Blocked by required conditions
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 21m37s
🏗️ Build and Test JellyTau / Supply Chain (push) Successful in 2m55s
Traceability Validation / Check Requirement Traces (push) Successful in 23s
Build & Release / Run Tests (push) Successful in 15m34s
Build & Release / Build Android (push) Waiting to run
🏗️ Build and Test JellyTau / Android Compile Check (push) Successful in 4m26s
Build & Release / Build Linux (push) Successful in 20m58s
Build & Release / Build Windows (push) In progress
Verified each against the code before acting; four of the five findings held, one did not. DR-253 — a deferred seek outlived its file. `seek` holds a position while MPV has nothing loaded and `FileLoaded` applies it (DR-241), but neither `load` nor `stop` discarded it. Scrub near the end of a transcoded item — which re-opens the stream — then skip to the next item before the reload completes, and the old position lands on the new item. It starts wherever the previous one was scrubbed to, silently. Both lifecycle points clear it now. DR-254 — a per-playback quality ceiling outlived its playback. The override is process-wide and describes one playback: dropping to 720p for a struggling episode says nothing about the next. Every advance the frontend drives clears it through player_play_item, but the background audio-only advance loads the next episode in Rust and skipped all three clearing sites — so every later episode stayed capped, with nothing in the UI explaining why. DR-255 — `playable_url` was a byte-identical copy of `playback_url`, added for the cross-platform open path. The original is `#[cfg(target_os = "android")]`, so it does not exist in a Linux build and nothing warned. Two matches over MediaSource meant a new variant could be handled in one and forgotten in the other. The gate is gone and the copy with it. The fifth finding — that the comment on `video_audio_codecs` describes a renderer switch the code no longer has — does not hold. `get_player_status` hard-codes Android to Native, but `experimentalNativeVideo` is still live in VideoPlayer.svelte as a suppressor that can force HTML5 even when Rust says native. The switch exists, so the narrow codec list is still doing its job. Both correctness fixes are red-then-green. The tests are wiring assertions in the style of UT-218: what matters is the call site, and reaching these at runtime needs a live MPV handle or a repository, a server and a player. That technique now appears three times and is worth watching — it pins call sites, not behaviour. The review's sharpest point is one it raised as redundancy: MpvPlayer already handles DR-253 correctly, resetting deferred state on every open, and the old path had to be patched separately. That is the drift two parallel engines produce, and the argument for finishing DR-248/249 rather than leaving LegacyPlayer in place indefinitely. 795 Rust tests, 1088 frontend, every CI check green locally.
435 lines
16 KiB
Rust
435 lines
16 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;
|
|
|
|
/// Every property the event loop *handles* must also be *observed*.
|
|
///
|
|
/// libmpv only delivers `PropertyChange` for properties registered with
|
|
/// `mpv_observe_property`. A `match` arm for an unobserved property is
|
|
/// unreachable code that looks exactly like working code: the handler is
|
|
/// right there, so the behaviour reads as implemented.
|
|
///
|
|
/// This cost a real bug. `pause` was handled and never observed, so
|
|
/// `StateChanged` was never emitted on pause or resume. It stayed invisible
|
|
/// while Linux video played in the webview, because the `<video>` element's
|
|
/// own DOM events drove the play/pause control; turning native video on made
|
|
/// the UI depend on the event that never came, and the button stopped
|
|
/// responding.
|
|
///
|
|
/// Asserted against the source because there is no way to observe the
|
|
/// registration at runtime without a live mpv instance.
|
|
///
|
|
/// TRACES: UR-005 | DR-239 | UT-218
|
|
#[test]
|
|
fn test_every_handled_property_is_observed() {
|
|
let src = include_str!("mpv_backend.rs");
|
|
|
|
let handled: Vec<&str> = src
|
|
.match_indices("PropertyChange { name: \"")
|
|
.filter_map(|(i, m)| {
|
|
let rest = &src[i + m.len()..];
|
|
rest.find('"').map(|end| &rest[..end])
|
|
})
|
|
.collect();
|
|
|
|
assert!(
|
|
!handled.is_empty(),
|
|
"no PropertyChange arms found - has the event loop been restructured?"
|
|
);
|
|
|
|
for name in handled {
|
|
let observed = format!("observe_property(\"{name}\"");
|
|
assert!(
|
|
src.contains(&observed),
|
|
"mpv_backend.rs handles PropertyChange for {name:?} but never calls \
|
|
observe_property({name:?}, ..). libmpv will never deliver that event, \
|
|
so the handler is dead code."
|
|
);
|
|
}
|
|
}
|
|
|
|
/// A deferred seek belongs to the file it was issued against.
|
|
///
|
|
/// `seek` holds a position when MPV has nothing loaded yet, and the
|
|
/// `FileLoaded` handler applies it (DR-241). Nothing discarded it when a
|
|
/// *different* file was loaded or playback stopped — so scrubbing near the
|
|
/// end of a transcoded item (which re-opens the stream) and then skipping to
|
|
/// the next item before the reload completed applied the old position to the
|
|
/// new item. It silently started wherever you had scrubbed to in the
|
|
/// previous one.
|
|
///
|
|
/// Asserted against the source: the state lives behind a live MPV handle,
|
|
/// and constructing one needs libmpv and an audio device that CI cannot be
|
|
/// assumed to have. Crude, but it pins the one thing that matters — that
|
|
/// both lifecycle points discard it.
|
|
///
|
|
/// TRACES: UR-040, UR-005 | DR-253 | UT-225
|
|
#[test]
|
|
fn test_load_and_stop_discard_a_deferred_seek() {
|
|
let src = include_str!("mpv_backend.rs");
|
|
|
|
for func in ["fn load(", "fn stop("] {
|
|
let start = src
|
|
.find(func)
|
|
.unwrap_or_else(|| panic!("{func} not found - has the backend been restructured?"));
|
|
// The body runs to the next top-level ` fn ` at the same depth.
|
|
let rest = &src[start + func.len()..];
|
|
let end = rest.find("\n fn ").unwrap_or(rest.len());
|
|
let body = &rest[..end];
|
|
|
|
assert!(
|
|
body.contains("pending_seek"),
|
|
"{func} does not discard `pending_seek`. A seek held for a file \
|
|
that is no longer loading will be applied to whatever loads next."
|
|
);
|
|
}
|
|
}
|
|
|
|
/// 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
|
|
);
|
|
}
|
|
}
|