From 90d55cd72ce653315b4915b7c6a295c58785f441 Mon Sep 17 00:00:00 2001 From: Kentaro Hakase Date: Wed, 12 Aug 2026 00:49:30 +0200 Subject: [PATCH] Cover the composed resource keys, the commands and the tool discovery Several texts are looked up under a key composed from an enum value, so adding an enum member shipped a missing-resource marker to the user with no compiler error and no test to stop it. The existing resource test walked a hand written list of about sixty values and could not catch that. A new test enumerates every member of every enum used that way and resolves the keys it produces, in both languages. Verified by adding a member without a resource entry: the test names exactly the two keys that are missing. The command types had no tests at all although they carry the guard that keeps a failing command from ending the session. Reentry, the error handler, the cancellation path and the typed parameter handling are covered now, as is Result, whose failure case may legitimately still carry a value. Tool discovery had no tests either. Path resolution, the fallback to the bare name, the caching of a successful probe and the new caching of a failed one are covered. The update check gets its first tests through the constructor that takes an HttpClient specifically for that purpose and was unused until now; it also propagates a cancellation instead of reporting it as no update available. Test classes no longer run in parallel: several change the process wide localization culture while others assert on localized text. --- .../AppUpdateServiceTests.cs | 118 +++++++++++ AudioQualityEnhancer.Tests/AssemblyInfo.cs | 5 + AudioQualityEnhancer.Tests/CommandTests.cs | 185 ++++++++++++++++++ AudioQualityEnhancer.Tests/ResourceTests.cs | 40 ++++ AudioQualityEnhancer.Tests/ResultTests.cs | 61 ++++++ .../ToolDiscoveryServiceTests.cs | 107 ++++++++++ CHANGELOG.md | 1 + Services/AppUpdateService.cs | 6 + 8 files changed, 523 insertions(+) create mode 100644 AudioQualityEnhancer.Tests/AssemblyInfo.cs create mode 100644 AudioQualityEnhancer.Tests/CommandTests.cs create mode 100644 AudioQualityEnhancer.Tests/ResultTests.cs create mode 100644 AudioQualityEnhancer.Tests/ToolDiscoveryServiceTests.cs diff --git a/AudioQualityEnhancer.Tests/AppUpdateServiceTests.cs b/AudioQualityEnhancer.Tests/AppUpdateServiceTests.cs index 8c66edb..83530af 100644 --- a/AudioQualityEnhancer.Tests/AppUpdateServiceTests.cs +++ b/AudioQualityEnhancer.Tests/AppUpdateServiceTests.cs @@ -1,3 +1,6 @@ +using System.Net; +using System.Net.Http; +using System.Text; using AudioQualityEnhancer.Services; namespace AudioQualityEnhancer.Tests; @@ -40,4 +43,119 @@ public void IsNewer_ComparesMajorMinorPatch(string current, string latest, bool { Assert.Equal(expected, AppUpdateService.IsNewer(Version.Parse(current), Version.Parse(latest))); } + + [Fact] + public async Task CheckAsync_ReportsANewerRelease() + { + const string releaseUrl = "https://github.com/Kentarohakase/AudioQualityEnhancer/releases/tag/v0.18.0"; + using var client = CreateClient(Release("v0.18.0", releaseUrl)); + + var update = await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None); + + Assert.NotNull(update); + Assert.Equal("0.18.0", update!.Version); + Assert.Equal(releaseUrl, update.Url); + } + + [Fact] + public async Task CheckAsync_ReturnsNullWhenTheCurrentVersionIsUpToDate() + { + using var client = CreateClient(Release("v0.17.0", "https://github.com/Kentarohakase/AudioQualityEnhancer/releases/tag/v0.17.0")); + + Assert.Null(await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None)); + } + + [Fact] + public async Task CheckAsync_FallsBackToTheReleasesPageForAnUntrustedLink() + { + using var client = CreateClient(Release("v0.18.0", "http://example.com/download.exe")); + + var update = await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None); + + Assert.NotNull(update); + Assert.Equal(AppUpdateService.ReleasesPageUrl, update!.Url); + } + + [Fact] + public async Task CheckAsync_ReturnsNullForAnErrorResponse() + { + using var client = CreateClient(new HttpResponseMessage(HttpStatusCode.NotFound)); + + Assert.Null(await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None)); + } + + [Fact] + public async Task CheckAsync_ReturnsNullForAMalformedPayload() + { + using var client = CreateClient(Json("not json at all")); + + Assert.Null(await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None)); + } + + [Fact] + public async Task CheckAsync_ReturnsNullWhenTheTagIsMissing() + { + using var client = CreateClient(Json("""{"html_url":"https://github.com/Kentarohakase/AudioQualityEnhancer"}""")); + + Assert.Null(await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None)); + } + + /// A cancelled check has to surface as a cancellation, not as "no update". + [Fact] + public async Task CheckAsync_PropagatesCancellation() + { + using var cancellation = new CancellationTokenSource(); + var handler = new FakeHttpMessageHandler(_ => + { + cancellation.Cancel(); + throw new OperationCanceledException(cancellation.Token); + }); + + using var client = new HttpClient(handler); + + await Assert.ThrowsAnyAsync( + () => new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), cancellation.Token)); + } + + [Fact] + public async Task CheckAsync_ReturnsNullWhenTheHostIsUnreachable() + { + using var client = new HttpClient(new FakeHttpMessageHandler(_ => throw new HttpRequestException("offline"))); + + Assert.Null(await new AppUpdateService(client).CheckAsync(new Version(0, 17, 0), CancellationToken.None)); + } + + private static HttpClient CreateClient(HttpResponseMessage response) + { + return new HttpClient(new FakeHttpMessageHandler(_ => response)); + } + + private static HttpResponseMessage Release(string tag, string url) + { + return Json($$"""{"tag_name":"{{tag}}","html_url":"{{url}}"}"""); + } + + private static HttpResponseMessage Json(string payload) + { + return new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(payload, Encoding.UTF8, "application/json") + }; + } + + private sealed class FakeHttpMessageHandler : HttpMessageHandler + { + private readonly Func _respond; + + public FakeHttpMessageHandler(Func respond) + { + _respond = respond; + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + return Task.FromResult(_respond(request)); + } + } } diff --git a/AudioQualityEnhancer.Tests/AssemblyInfo.cs b/AudioQualityEnhancer.Tests/AssemblyInfo.cs new file mode 100644 index 0000000..92460ed --- /dev/null +++ b/AudioQualityEnhancer.Tests/AssemblyInfo.cs @@ -0,0 +1,5 @@ +// LocalizationService.Instance.Culture is process wide state, and its setter also writes +// CultureInfo.DefaultThreadCurrentUICulture. Several test classes change it while others +// assert on localized text, so running classes in parallel makes those assertions depend +// on timing. The whole suite takes a few seconds, so serializing it is the cheap fix. +[assembly: CollectionBehavior(DisableTestParallelization = true)] diff --git a/AudioQualityEnhancer.Tests/CommandTests.cs b/AudioQualityEnhancer.Tests/CommandTests.cs new file mode 100644 index 0000000..93ea468 --- /dev/null +++ b/AudioQualityEnhancer.Tests/CommandTests.cs @@ -0,0 +1,185 @@ +using AudioQualityEnhancer.ViewModels; + +namespace AudioQualityEnhancer.Tests; + +public sealed class CommandTests +{ + [Fact] + public void AsyncRelayCommand_WithoutPredicate_CanExecute() + { + var command = new AsyncRelayCommand(() => Task.CompletedTask); + + Assert.True(command.CanExecute(null)); + } + + [Fact] + public void AsyncRelayCommand_HonoursItsPredicate() + { + var allowed = false; + var command = new AsyncRelayCommand(() => Task.CompletedTask, () => allowed); + + Assert.False(command.CanExecute(null)); + + allowed = true; + Assert.True(command.CanExecute(null)); + } + + [Fact] + public async Task AsyncRelayCommand_BlocksReentryWhileRunning() + { + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var executions = 0; + var command = new AsyncRelayCommand(async () => + { + executions++; + await gate.Task; + }); + + command.Execute(null); + Assert.False(command.CanExecute(null)); + + // A second click while the first run is still in flight must be ignored. + command.Execute(null); + Assert.Equal(1, executions); + + gate.SetResult(); + await WaitUntilAsync(() => command.CanExecute(null)); + Assert.Equal(1, executions); + } + + [Fact] + public async Task AsyncRelayCommand_ReportsFailureToTheErrorHandler() + { + Exception? reported = null; + var command = new AsyncRelayCommand( + () => throw new InvalidOperationException("boom"), + onError: exception => reported = exception); + + command.Execute(null); + await WaitUntilAsync(() => reported is not null); + + Assert.IsType(reported); + Assert.Equal("boom", reported!.Message); + } + + [Fact] + public async Task AsyncRelayCommand_TreatsCancellationAsANormalOutcome() + { + Exception? reported = null; + var command = new AsyncRelayCommand( + () => throw new OperationCanceledException(), + onError: exception => reported = exception); + + command.Execute(null); + await WaitUntilAsync(() => command.CanExecute(null)); + + Assert.Null(reported); + } + + [Fact] + public async Task AsyncRelayCommand_BecomesExecutableAgainAfterAFailure() + { + var command = new AsyncRelayCommand( + () => throw new InvalidOperationException(), + onError: _ => { }); + + command.Execute(null); + await WaitUntilAsync(() => command.CanExecute(null)); + + Assert.True(command.CanExecute(null)); + } + + [Fact] + public void AsyncRelayCommand_DoesNotRunWhenItCannotExecute() + { + var executions = 0; + var command = new AsyncRelayCommand( + () => + { + executions++; + return Task.CompletedTask; + }, + () => false); + + command.Execute(null); + + Assert.Equal(0, executions); + } + + [Fact] + public void RelayCommand_RaisesCanExecuteChanged() + { + var raised = 0; + var command = new RelayCommand(() => { }); + command.CanExecuteChanged += (_, _) => raised++; + + command.RaiseCanExecuteChanged(); + + Assert.Equal(1, raised); + } + + [Fact] + public void RelayCommand_ExecutesAndHonoursItsPredicate() + { + var executions = 0; + var allowed = true; + var command = new RelayCommand(() => executions++, () => allowed); + + Assert.True(command.CanExecute(null)); + command.Execute(null); + Assert.Equal(1, executions); + + allowed = false; + Assert.False(command.CanExecute(null)); + } + + [Fact] + public void RelayCommandOfT_PassesTheTypedParameter() + { + string? received = null; + var command = new RelayCommand(value => received = value); + + command.Execute("preset"); + + Assert.Equal("preset", received); + } + + [Fact] + public void RelayCommandOfT_PassesDefaultForAMismatchedParameter() + { + var executions = 0; + string? received = "unchanged"; + var command = new RelayCommand(value => + { + executions++; + received = value; + }); + + command.Execute(42); + + Assert.Equal(1, executions); + Assert.Null(received); + } + + [Fact] + public void RelayCommandOfT_EvaluatesThePredicateForNullAndForAValue() + { + var command = new RelayCommand(_ => { }, value => value is not null); + + Assert.False(command.CanExecute(null)); + Assert.True(command.CanExecute("preset")); + + // A parameter of a different type is neither T nor null, so it cannot execute. + Assert.False(command.CanExecute(42)); + } + + private static async Task WaitUntilAsync(Func condition) + { + for (var attempt = 0; attempt < 100 && !condition(); attempt++) + { + await Task.Delay(10); + } + + Assert.True(condition(), "The condition was not met in time."); + } +} diff --git a/AudioQualityEnhancer.Tests/ResourceTests.cs b/AudioQualityEnhancer.Tests/ResourceTests.cs index 35bf4bd..ec1bbc6 100644 --- a/AudioQualityEnhancer.Tests/ResourceTests.cs +++ b/AudioQualityEnhancer.Tests/ResourceTests.cs @@ -18,6 +18,39 @@ public void GermanAndEnglishResources_ExposeTheSameKeys() Assert.Empty(englishKeys.Except(germanKeys)); } + /// + /// Several texts are looked up under a key that is composed from an enum value, so a + /// new enum member ships "!Key!" to the user with no compiler error to stop it. This + /// walks every member of every enum used that way and resolves the keys it produces. + /// + [Theory] + [InlineData("de")] + [InlineData("en")] + public void ComposedResourceKeys_ResolveForEveryEnumValue(string cultureName) + { + LocalizationService.Instance.Culture = CultureInfo.GetCultureInfo(cultureName); + + var keys = new List(); + keys.AddRange(ComposeKeys("AnalysisStatus_{0}", "AnalysisSummary_{0}")); + keys.AddRange(ComposeKeys("AnalysisSeverity_{0}")); + keys.AddRange(ComposeKeys("AnalysisFinding_{0}_Title", "AnalysisFinding_{0}_Message")); + keys.AddRange(ComposeKeys("ValidationStatus_{0}", "ValidationSummary_{0}")); + keys.AddRange(ComposeKeys("ValidationFinding_{0}_Title", "ValidationFinding_{0}_Message")); + keys.AddRange(ComposeKeys("BatchStatus_{0}")); + + // A recommendation only accompanies an actual finding, so the "no issues" kind + // deliberately has no recommendation text. + keys.AddRange(Enum.GetValues() + .Where(kind => kind != AudioAnalysisFindingKind.NoIssues) + .Select(kind => $"AnalysisRecommendation_{kind}")); + + var unresolved = keys + .Where(key => MissingResourceRegex().IsMatch(LocalizationService.Instance[key])) + .ToArray(); + + Assert.Empty(unresolved); + } + [Theory] [InlineData("de")] [InlineData("en")] @@ -150,6 +183,13 @@ public void VisibleModelTexts_DoNotContainMissingResourceMarkers(string cultureN } } + private static IEnumerable ComposeKeys(params string[] formats) + where TEnum : struct, Enum + { + return Enum.GetValues() + .SelectMany(value => formats.Select(format => string.Format(CultureInfo.InvariantCulture, format, value))); + } + private static IReadOnlySet LoadResourceKeys(string fileName) { var path = Path.Combine(TestPaths.RepositoryRoot, "Resources", fileName); diff --git a/AudioQualityEnhancer.Tests/ResultTests.cs b/AudioQualityEnhancer.Tests/ResultTests.cs new file mode 100644 index 0000000..eec93ab --- /dev/null +++ b/AudioQualityEnhancer.Tests/ResultTests.cs @@ -0,0 +1,61 @@ +using AudioQualityEnhancer.Models; + +namespace AudioQualityEnhancer.Tests; + +public sealed class ResultTests +{ + [Fact] + public void Success_HasNoErrorMessageAndNoException() + { + var result = Result.Success(); + + Assert.True(result.IsSuccess); + Assert.False(result.IsFailure); + Assert.Null(result.ErrorMessage); + Assert.Null(result.Exception); + } + + [Fact] + public void Failure_KeepsMessageAndException() + { + var exception = new InvalidOperationException("boom"); + var result = Result.Failure("failed", exception); + + Assert.True(result.IsFailure); + Assert.Equal("failed", result.ErrorMessage); + Assert.Same(exception, result.Exception); + } + + [Fact] + public void SuccessOfT_CarriesTheValue() + { + var result = Result.Success("output.flac"); + + Assert.True(result.IsSuccess); + Assert.Equal("output.flac", result.Value); + } + + /// + /// A failing step can still produce a partial result - result validation attaches the + /// report of a critical comparison, for instance. Consumers therefore have to check + /// the flag rather than assume a value means success. + /// + [Fact] + public void FailureOfT_CanStillCarryAValue() + { + var result = Result.Failure("critical", value: "output.flac"); + + Assert.True(result.IsFailure); + Assert.Equal("critical", result.ErrorMessage); + Assert.Equal("output.flac", result.Value); + } + + [Fact] + public void FailureOfT_WithoutAValueYieldsTheDefault() + { + var result = Result.Failure("failed"); + + Assert.True(result.IsFailure); + Assert.Null(result.Value); + } +} diff --git a/AudioQualityEnhancer.Tests/ToolDiscoveryServiceTests.cs b/AudioQualityEnhancer.Tests/ToolDiscoveryServiceTests.cs new file mode 100644 index 0000000..4e3558a --- /dev/null +++ b/AudioQualityEnhancer.Tests/ToolDiscoveryServiceTests.cs @@ -0,0 +1,107 @@ +using AudioQualityEnhancer.Services; + +namespace AudioQualityEnhancer.Tests; + +public sealed class ToolDiscoveryServiceTests +{ + // where.exe is always on PATH on Windows and "/?" makes it print its help and exit 0, + // which is the same shape as the " -version" probe without needing FFmpeg. + // (cmd.exe is not usable here: "cmd /?" prints help but exits with 1.) + private const string PathTool = "where"; + private const string PathToolProbeArgument = "/?"; + private const string MissingTool = "audioqualityenhancer-missing-tool"; + + [Fact] + public void ResolveExecutable_ReturnsTheFullPathForAToolOnPath() + { + var service = new ToolDiscoveryService(); + + var path = service.ResolveExecutable(PathTool); + + Assert.True(Path.IsPathFullyQualified(path)); + Assert.True(File.Exists(path)); + Assert.Equal("where.exe", Path.GetFileName(path)); + } + + /// + /// An unresolvable tool falls back to the bare executable name so the later process + /// start produces the normal "not found" error instead of an empty command line. + /// + [Fact] + public void ResolveExecutable_FallsBackToTheBareNameForAnUnknownTool() + { + var service = new ToolDiscoveryService(); + + Assert.Equal($"{MissingTool}.exe", service.ResolveExecutable(MissingTool)); + } + + [Fact] + public void ResolveExecutable_CachesTheLocation() + { + var service = new ToolDiscoveryService(); + + Assert.Equal(service.ResolveExecutable(PathTool), service.ResolveExecutable(PathTool)); + } + + [Fact] + public async Task GetStatusAsync_ReportsAToolFoundOnPath() + { + var service = new ToolDiscoveryService(); + + var status = await service.GetStatusAsync(PathTool, CancellationToken.None, PathToolProbeArgument); + + Assert.True(status.IsAvailable); + Assert.Equal(PathTool, status.Name); + Assert.Equal("ToolSource_Path", status.SourceKey); + Assert.False(string.IsNullOrWhiteSpace(status.VersionLine)); + Assert.Null(status.ErrorMessage); + } + + [Fact] + public async Task GetStatusAsync_CachesASuccessfulProbe() + { + var service = new ToolDiscoveryService(); + + var first = await service.GetStatusAsync(PathTool, CancellationToken.None, PathToolProbeArgument); + var second = await service.GetStatusAsync(PathTool, CancellationToken.None, PathToolProbeArgument); + + Assert.Same(first, second); + } + + [Fact] + public async Task GetStatusAsync_ReportsAMissingToolAsUnavailable() + { + var service = new ToolDiscoveryService(); + + var status = await service.GetStatusAsync(MissingTool, CancellationToken.None); + + Assert.False(status.IsAvailable); + Assert.Equal(MissingTool, status.Name); + Assert.False(string.IsNullOrWhiteSpace(status.ErrorMessage)); + } + + /// + /// Probing is bounded by a timeout, so repeating it for a tool that is missing or does + /// not answer would cost that timeout on every call. The failure is cached instead. + /// + [Fact] + public async Task GetStatusAsync_CachesAFailedProbe() + { + var service = new ToolDiscoveryService(); + + var first = await service.GetStatusAsync(MissingTool, CancellationToken.None); + var second = await service.GetStatusAsync(MissingTool, CancellationToken.None); + + Assert.Same(first, second); + } + + [Fact] + public void FailureCache_ExpiresSoonEnoughToPickUpAnInstalledTool() + { + // A tool installed while the app runs has to be picked up again, so the failure + // must not be remembered for the whole session - but long enough that a repeated + // status query does not pay the probe timeout again. + Assert.True(ToolDiscoveryService.FailedStatusCacheDuration > TimeSpan.Zero); + Assert.True(ToolDiscoveryService.FailedStatusCacheDuration <= TimeSpan.FromMinutes(5)); + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e9750b..f385792 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - The queue no longer re-evaluates every button and rebuilds the summary on each FFmpeg progress tick, so the window stays responsive during a batch export. - The startup checks for yt-dlp and for a new release are stopped when the window closes and no longer write into a closed window; an unexpected failure in either is logged instead of being swallowed. - A tool that is missing or does not answer is remembered for half a minute instead of being probed again on every call, where each attempt cost the full probe timeout. +- A cancelled update check is reported as a cancellation instead of as "no update available". ## 0.17.0 - 2026-06-13 diff --git a/Services/AppUpdateService.cs b/Services/AppUpdateService.cs index 3e07e62..532c03c 100644 --- a/Services/AppUpdateService.cs +++ b/Services/AppUpdateService.cs @@ -58,6 +58,12 @@ internal AppUpdateService(HttpClient httpClient) var url = root.TryGetProperty("html_url", out var urlElement) ? urlElement.GetString() : null; return new AppUpdateInfo(latest.ToString(), ResolveReleaseUrl(url)); } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // A caller that cancels wants to know it was cancelled, not that there is + // no update. Its own timeout still surfaces as "no update" below. + throw; + } catch { // Update checks are best effort and must never disrupt startup (offline, rate limit, ...).