ErsatzTV.Scanner.Tests/Application/MediaSources/ScanLocalLibraryHandlerTests.cs

@@ -0,0 +1,186 @@
+using ErsatzTV.Core;
+using ErsatzTV.Core.Domain;
+using ErsatzTV.Core.Interfaces.Repositories;
+using ErsatzTV.Scanner.Application.MediaSources;
+using ErsatzTV.Scanner.Core.Interfaces;
+using ErsatzTV.Scanner.Core.Interfaces.Metadata;
+using Microsoft.Extensions.Logging;
+using NSubstitute;
+using NUnit.Framework;
+using Shouldly;
+
+namespace ErsatzTV.Scanner.Tests.Application.MediaSources;
+
+public class ScanLocalLibraryHandlerTests
+{
+    [TestFixture]
+    public class Handle
+    {
+        [SetUp]
+        public void SetUp()
+        {
+            // the handler validates that the configured ffmpeg/ffprobe paths exist on disk
+            _ffmpegPath = Path.GetTempFileName();
+            _ffprobePath = Path.GetTempFileName();
+
+            _scannerProxy = Substitute.For<IScannerProxy>();
+            _libraryRepository = Substitute.For<ILibraryRepository>();
+            _configElementRepository = Substitute.For<IConfigElementRepository>();
+            _movieFolderScanner = Substitute.For<IMovieFolderScanner>();
+            _logger = Substitute.For<ILogger<ScanLocalLibraryHandler>>();
+
+            _library = new LocalLibrary
+            {
+                Id = 42,
+                Name = "Movies",
+                MediaKind = LibraryMediaKind.Movies,
+                MediaSourceId = 1,
+                Paths = [new LibraryPath { Id = 1, Path = "/movies" }]
+            };
+
+            _libraryRepository.GetLibrary(_library.Id).Returns(Some<Library>(_library).AsTask());
+
+            ConfigValue(ConfigElementKey.FFmpegPath, _ffmpegPath);
+            ConfigValue(ConfigElementKey.FFprobePath, _ffprobePath);
+            ConfigValue(ConfigElementKey.LibraryRefreshInterval, 0);
+        }
+
+        [TearDown]
+        public void TearDown()
+        {
+            File.Delete(_ffmpegPath);
+            File.Delete(_ffprobePath);
+        }
+
+        private string _ffmpegPath;
+        private string _ffprobePath;
+        private IScannerProxy _scannerProxy;
+        private ILibraryRepository _libraryRepository;
+        private IConfigElementRepository _configElementRepository;
+        private IMovieFolderScanner _movieFolderScanner;
+        private ILogger<ScanLocalLibraryHandler> _logger;
+        private LocalLibrary _library;
+
+        [Test]
+        public async Task Should_Set_Library_LastScan_After_Successful_Scan()
+        {
+            ScanResult(Right<BaseError, Unit>(Unit.Default));
+
+            Either<BaseError, string> result = await Handler().Handle(
+                new ScanLocalLibrary("http://ersatztv.example", _library.Id, true),
+                CancellationToken.None);
+
+            result.IsRight.ShouldBeTrue();
+
+            // the regression: only the path-level LastScan was written, so the API's
+            // library-level scan time stayed null and the SPA showed "Never scanned" forever
+            _library.LastScan.ShouldNotBeNull();
+            await _libraryRepository.Received(1).UpdateLastScan(_library);
+            await _libraryRepository.Received(1).UpdateLastScan(_library.Paths[0]);
+        }
+
+        [Test]
+        public async Task Should_Not_Set_Library_LastScan_When_A_Path_Scan_Fails()
+        {
+            ScanResult(Left<BaseError, Unit>(new BaseError("scan failed")));
+
+            await Handler().Handle(
+                new ScanLocalLibrary("http://ersatztv.example", _library.Id, true),
+                CancellationToken.None);
+
+            _library.LastScan.ShouldBeNull();
+            await _libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any<Library>());
+
+            // a failed path suppresses the library-level scan time, reproducing the #264 symptom;
+            // without this log the user would have nothing explaining why
+            ShouldHaveLogged("Error scanning local library path /movies: scan failed");
+        }
+
+        [Test]
+        public async Task Should_Not_Set_Library_LastScan_When_A_Later_Path_Fails()
+        {
+            var goodPath = new LibraryPath { Id = 1, Path = "/movies" };
+            var badPath = new LibraryPath { Id = 2, Path = "/more-movies" };
+            _library.Paths = [goodPath, badPath];
+
+            _movieFolderScanner.ScanFolder(
+                    goodPath,
+                    _ffmpegPath,
+                    _ffprobePath,
+                    Arg.Any<decimal>(),
+                    Arg.Any<decimal>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(Right<BaseError, Unit>(Unit.Default).AsTask());
+            _movieFolderScanner.ScanFolder(
+                    badPath,
+                    _ffmpegPath,
+                    _ffprobePath,
+                    Arg.Any<decimal>(),
+                    Arg.Any<decimal>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(Left<BaseError, Unit>(new BaseError("scan failed")).AsTask());
+
+            await Handler().Handle(
+                new ScanLocalLibrary("http://ersatztv.example", _library.Id, true),
+                CancellationToken.None);
+
+            // a partially-scanned library must not claim a successful scan time
+            _library.LastScan.ShouldBeNull();
+            await _libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any<Library>());
+            await _libraryRepository.Received(1).UpdateLastScan(goodPath);
+        }
+
+        [Test]
+        public async Task Should_Not_Set_Library_LastScan_When_Scan_Is_Skipped()
+        {
+            ScanResult(Right<BaseError, Unit>(Unit.Default));
+            _library.Paths[0].LastScan = DateTime.UtcNow;
+            ConfigValue(ConfigElementKey.LibraryRefreshInterval, 6);
+
+            await Handler().Handle(
+                new ScanLocalLibrary("http://ersatztv.example", _library.Id, false),
+                CancellationToken.None);
+
+            // nothing was scanned, so there is no new scan time to record
+            _library.LastScan.ShouldBeNull();
+            await _libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any<Library>());
+        }
+
+        private void ConfigValue<T>(ConfigElementKey key, T value) =>
+            _configElementRepository.GetValue<T>(
+                    Arg.Is<ConfigElementKey>(k => k.Key == key.Key),
+                    Arg.Any<CancellationToken>())
+                .Returns(Task.FromResult<Option<T>>(Some(value)));
+
+        private void ScanResult(Either<BaseError, Unit> result) =>
+            _movieFolderScanner.ScanFolder(
+                    Arg.Any<LibraryPath>(),
+                    _ffmpegPath,
+                    _ffprobePath,
+                    Arg.Any<decimal>(),
+                    Arg.Any<decimal>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(result.AsTask());
+
+        private ScanLocalLibraryHandler Handler() =>
+            new(
+                _scannerProxy,
+                _libraryRepository,
+                _configElementRepository,
+                _movieFolderScanner,
+                Substitute.For<ITelevisionFolderScanner>(),
+                Substitute.For<IMusicVideoFolderScanner>(),
+                Substitute.For<IOtherVideoFolderScanner>(),
+                Substitute.For<ISongFolderScanner>(),
+                Substitute.For<IImageFolderScanner>(),
+                Substitute.For<IRemoteStreamFolderScanner>(),
+                _logger);
+
+        // asserts on the substituted ILogger.Log call the LogError extension dispatches to
+        // (same idiom as ErsatzTV.Tests' ReleaseMemoryHandlerTests.ShouldHaveLogged)
+        private void ShouldHaveLogged(string expectedMessage) =>
+            _logger.ReceivedCalls()
+                .Any(call => call.GetArguments().ElementAtOrDefault(2)?.ToString() == expectedMessage)
+                .ShouldBeTrue();
+    }
+}

ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs

@@ -1,4 +1,4 @@
-using System.Diagnostics;
+using System.Diagnostics;
 using ErsatzTV.Core;
 using ErsatzTV.Core.Domain;
 using ErsatzTV.Core.Interfaces.Repositories;
@@ -65,6 +65,7 @@ public class ScanLocalLibraryHandler : IRequestHandler<ScanLocalLibrary, Either<
         _scannerProxy.SetBaseUrl(baseUrl);
 
         var scanned = false;
+        var anyFailed = false;
 
         for (var i = 0; i < localLibrary.Paths.Count; i++)
         {
@@ -145,6 +146,21 @@ public class ScanLocalLibraryHandler : IRequestHandler<ScanLocalLibrary, Either<
                     libraryPath.LastScan = DateTime.UtcNow;
                     await _libraryRepository.UpdateLastScan(libraryPath);
                 }
+                else
+                {
+                    anyFailed = true;
+
+                    // a failed path now suppresses the library-level scan time below, so without this the
+                    // user sees "Never scanned" with nothing explaining why. The remote scanners log the
+                    // same way.
+                    foreach (BaseError error in result.LeftToSeq())
+                    {
+                        _logger.LogError(
+                            "Error scanning local library path {Path}: {Error}",
+                            libraryPath.Path,
+                            error);
+                    }
+                }
             }
 
             await _scannerProxy.UpdateProgress(progressMax, cancellationToken);
@@ -152,6 +168,15 @@ public class ScanLocalLibraryHandler : IRequestHandler<ScanLocalLibrary, Either<
 
         sw.Stop();
 
+        // the library-level LastScan drives the "last scanned" / "Never scanned" badge in the API and SPA,
+        // and is separate from the per-path LastScan that gates the refresh interval above. Only record it
+        // when every path that ran succeeded, mirroring the remote scanners (which set it solely on IsRight).
+        if (scanned && !anyFailed)
+        {
+            localLibrary.LastScan = DateTime.UtcNow;
+            await _libraryRepository.UpdateLastScan(localLibrary);
+        }
+
         if (scanned)
         {
             _logger.LogDebug(

docs/testing.md

@@ -9,6 +9,7 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points
 |---|---|---|
 | `ErsatzTV.Tests` | API controllers + MediatR handlers | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. 828 tests currently. |
 | `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. 542 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). |
+| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. 1471 tests. Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
 | `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. |
 | `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |
 | `web/` (vitest) | React SPA unit tests | 330 tests; run alongside typecheck + build (see below). |
@@ -85,10 +86,10 @@ npm run build      # tsc -b && vite build
 
 ## Per-PR verification gate
 
-Before opening a PR: build the solution, run both .NET test projects (plus
-`ErsatzTV.Architecture.Tests` and `ErsatzTV.FFmpeg.Tests` if touched), and run the web test/lint/
-typecheck/build steps above. All must be green. A golden-file diff or an architecture-test
-failure is a hard stop — fix the code, don't regenerate/relax the test.
+Before opening a PR: build the solution, run `ErsatzTV.Tests` + `ErsatzTV.Core.Tests` (plus
+`ErsatzTV.Scanner.Tests`, `ErsatzTV.Architecture.Tests` and `ErsatzTV.FFmpeg.Tests` if touched),
+and run the web test/lint/typecheck/build steps above. All must be green. A golden-file diff or
+an architecture-test failure is a hard stop — fix the code, don't regenerate/relax the test.
 
 ## See also