Strip comments and dedupe normalized literals in backend tests

- Remove all // and /// comments from the backend test suite. - Replace hardcoded normalized-form string literals (e.g. NormalizedTitle/ NormalizedName duplicating a title/name in upper case) with a variable plus .ToUpperInvariant(), so the two values can't drift apart.
2026-09-10 02:45:11 +02:00
parent 5658dd24ff
commit c418844178
14 changed files with 29 additions and 110 deletions
@@ -8,8 +8,6 @@ namespace Musify.Api.Tests.Extensions
 {
    public sealed class ErrorOrHttpExtensionsTests
    {
-        // The concrete IResult types (NoContent, UnauthorizedHttpResult, ...) resolve logging/problem-
-        // details services from HttpContext.RequestServices when they execute, so it can't be empty.
        private static readonly IServiceProvider Services = new ServiceCollection().AddLogging().AddProblemDetails().BuildServiceProvider();

        private static async Task<(int StatusCode, string Body)> ExecuteAsync(IResult result)
@@ -12,11 +12,6 @@ namespace Musify.Api.Tests.Filters
    {
        public sealed record Payload(string Name);

-        // EndpointFilterInvocationContext.Create has no (HttpContext, object[]) overload — only
-        // Create<T0>, Create<T0,T1>, etc. Passing an object[] to a params-object[] call site binds
-        // to Create<T0> with T0 inferred as object[] itself, so .Arguments ends up with one element
-        // (the array, boxed) instead of the array's contents — OfType<Payload>() then never matches
-        // and the filter silently falls through to "no argument, skip validation" for every case.
        private static EndpointFilterInvocationContext CreateContext<T>(T argument) =>
            EndpointFilterInvocationContext.Create(new DefaultHttpContext(), argument);

@@ -13,14 +13,6 @@ using Xunit;

 namespace Musify.Api.Tests.TestSupport
 {
-    /// <summary>
-    /// Boots the real <c>Musify.Api</c> host — real routing, real auth/authorization pipeline,
-    /// real validators, real Mediator handlers — against a private Postgres container (so EF
-    /// queries behave like production, not like a stand-in). The services this API talks to over
-    /// the network (object storage, YouTube, the message bus, stream-ticket signing) are replaced
-    /// with NSubstitute fakes: they are Infrastructure's job to verify against the real thing (see
-    /// Musify.Infrastructure.Tests), not the HTTP layer's.
-    /// </summary>
    public sealed class ApiTestFixture : WebApplicationFactory<Program>, IAsyncLifetime
    {
        private readonly PostgreSqlContainer postgres = new PostgreSqlBuilder("postgres:17-alpine")
@@ -46,20 +38,14 @@ namespace Musify.Api.Tests.TestSupport
            await database.Database.MigrateAsync();
        }

-        // xUnit v3's IAsyncLifetime IS System.IAsyncDisposable (rather than declaring its own
-        // Task-returning DisposeAsync as in v2), so there is a single DisposeAsync slot to fill —
-        // override the one WebApplicationFactory already provides instead of implementing the
-        // interface explicitly alongside it.
        public override async ValueTask DisposeAsync()
        {
            await base.DisposeAsync();
            await postgres.DisposeAsync();
        }

-        /// <summary>An <see cref="HttpClient"/> with no auth header — every protected endpoint answers 401.</summary>
        public HttpClient CreateAnonymousClient() => CreateClient();

-        /// <summary>An <see cref="HttpClient"/> authenticated as the given user id (see <see cref="FakeAuthenticationHandler"/>).</summary>
        public HttpClient CreateAuthenticatedClient(long userId, string userName = "test-user")
        {
            var client = CreateClient();
@@ -105,8 +91,6 @@ namespace Musify.Api.Tests.TestSupport
                services.Replace(ServiceDescriptor.Singleton(YouTubeMusicService));
                services.Replace(ServiceDescriptor.Singleton(StreamTicketService));

-                // Overrides the real JwtBearer scheme as the default, so requests authenticate via
-                // FakeAuthenticationHandler instead of needing a real token and a reachable IdP.
                services
                    .AddAuthentication(FakeAuthenticationHandler.SchemeName)
                    .AddScheme<AuthenticationSchemeOptions, FakeAuthenticationHandler>(FakeAuthenticationHandler.SchemeName, _ => { });
@@ -6,12 +6,6 @@ using Microsoft.Extensions.Options;

 namespace Musify.Api.Tests.TestSupport
 {
-    /// <summary>
-    /// Replaces the real JwtBearer handler in the test host: a request carrying the
-    /// <see cref="UserIdHeader"/> header authenticates as that user id, with no real token,
-    /// issuer or JWKS endpoint involved. A request without the header stays anonymous, so
-    /// endpoints under <c>.RequireAuthorization()</c> correctly answer 401.
-    /// </summary>
    public sealed class FakeAuthenticationHandler(
        IOptionsMonitor<AuthenticationSchemeOptions> options,
        ILoggerFactory logger,
@@ -78,9 +78,6 @@ namespace Musify.Application.Tests.Mixes
        [Fact]
        public async Task Handle_YouTubeSourceUnavailable_StillCreatesMixesFromLocalCandidatesOnly()
        {
-            // Simulates IYouTubeMusicService.SearchSongsAsync as it behaves when the YouTube source is
-            // disabled (DisabledYouTubeMusicService, see Infrastructure): every call fails. Mix
-            // generation must degrade to local-only candidates instead of failing the whole command.
            youTubeMusicService
                .SearchSongsAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
                .Returns(Error.Forbidden(description: "The YouTube Music source is disabled."));
@@ -26,8 +26,8 @@ namespace Musify.Application.Tests.Shared
        public void FromEntity_ExternalTrackWithArtists_JoinsThemInPositionOrder()
        {
            var track = TestEntities.ExternalTrack();
-            var second = new Artist { Name = "Second Artist", NormalizedName = "SECOND ARTIST", ExternalId = "second" };
-            var first = new Artist { Name = "First Artist", NormalizedName = "FIRST ARTIST", ExternalId = "first" };
+            var second = TestEntities.Artist("Second Artist", externalId: "second");
+            var first = TestEntities.Artist("First Artist", externalId: "first");
            track.TrackArtists =
            [
                new TrackArtist { TrackId = track.Id, ArtistId = second.Id, Position = 1, Artist = second, Track = track },
@@ -4,12 +4,6 @@ using Xunit;

 namespace Musify.Application.Tests.TestSupport
 {
-    /// <summary>
-    /// Base class for handler tests: gives every test method its own empty, isolated
-    /// <see cref="TestDatabase"/> (xUnit creates a new instance of the test class per test method,
-    /// so there is no state to reset between tests) and a no-op logger factory for handlers that
-    /// take an <see cref="ILogger{TCategoryName}"/>.
-    /// </summary>
    public abstract class HandlerTestBase : IAsyncLifetime
    {
        private TestDatabase database = null!;
@@ -22,7 +16,6 @@ namespace Musify.Application.Tests.TestSupport

        protected static ILogger<T> NoOpLogger<T>() => NullLogger<T>.Instance;

-        /// <summary>Adds the entities and persists them, so handlers under test see committed data.</summary>
        protected async Task SeedAsync(params object[] entities)
        {
            foreach (var entity in entities)
@@ -2,10 +2,6 @@ using Musify.Application.Configuration;

 namespace Musify.Application.Tests.TestSupport
 {
-    /// <summary>
-    /// Configuration POCOs with every <c>required</c> member filled and the rest left at their
-    /// production defaults, so handler tests don't have to know which fields matter for them.
-    /// </summary>
    public static class TestConfigurations
    {
        public static ApplicationStorageConfiguration Storage(string bucket = "test-bucket") =>
@@ -5,25 +5,6 @@ using Musify.Domain.Entities;

 namespace Musify.Application.Tests.TestSupport
 {
-    /// <summary>
-    /// An <see cref="IDatabase"/> backed by a private, in-memory SQLite database.
-    /// </summary>
-    /// <remarks>
-    /// <para>
-    /// <see cref="IDatabase"/> exposes raw <see cref="DbSet{TEntity}"/> properties, so handlers
-    /// compose real LINQ queries (<c>Where</c>, <c>Select</c>, <c>Include</c>, ...) against it.
-    /// Substituting <see cref="IDatabase"/> with a mock cannot express that — there is no
-    /// <see cref="IQueryable{T}"/> provider behind a mock. SQLite's in-memory mode gives us a real
-    /// relational engine instead: fast, no external dependency, and — unlike the EF Core InMemory
-    /// provider — it actually supports transactions, which two handlers rely on.
-    /// </para>
-    /// <para>
-    /// The model mirrors the parts of <c>Musify.Infrastructure.Persistence.Database</c> that affect
-    /// query shape (owned types, table-per-type inheritance, unique indexes). MassTransit outbox/saga
-    /// state and Postgres-specific configuration are left out — the Application layer never touches
-    /// them.
-    /// </para>
-    /// </remarks>
    public sealed class TestDatabase : DbContext, IDatabase
    {
        private readonly SqliteConnection connection;
@@ -34,11 +15,6 @@ namespace Musify.Application.Tests.TestSupport
            this.connection = connection;
        }

-        /// <summary>
-        /// Creates a fresh, empty database with its own private connection. Call <see cref="Seed"/>
-        /// (via <see cref="Seeding.EntitySeedExtensions"/>) to populate it, then <see cref="DisposeAsync"/>
-        /// when the test is done.
-        /// </summary>
        public static async Task<TestDatabase> CreateAsync()
        {
            var connection = new SqliteConnection("Filename=:memory:");
@@ -3,11 +3,6 @@ using Musify.Domain.ValueObjects;

 namespace Musify.Application.Tests.TestSupport
 {
-    /// <summary>
-    /// Builds fully-populated domain entities for tests, with sensible defaults for every
-    /// <c>required</c> member. Every factory always returns a valid instance — never null — so
-    /// callers never need a null check on what this class hands back.
-    /// </summary>
    public static class TestEntities
    {
        public static User User(
@@ -13,7 +13,8 @@ namespace Musify.Infrastructure.Tests.Persistence
            await using var database = fixture.CreateDatabase();
            var before = DateTime.UtcNow;

-            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = "audit-user", NormalizedName = "AUDIT-USER" };
+            var name = "audit-user";
+            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = name, NormalizedName = name.ToUpperInvariant() };
            await database.Users.AddAsync(user, TestContext.Current.CancellationToken);
            await database.SaveChangesAsync(TestContext.Current.CancellationToken);

@@ -28,7 +29,8 @@ namespace Musify.Infrastructure.Tests.Persistence
        {
            await using var database = fixture.CreateDatabase();

-            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = "audit-user-2", NormalizedName = "AUDIT-USER-2" };
+            var name = "audit-user-2";
+            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = name, NormalizedName = name.ToUpperInvariant() };
            await database.Users.AddAsync(user, TestContext.Current.CancellationToken);
            await database.SaveChangesAsync(TestContext.Current.CancellationToken);
            var originalCreatedAt = user.CreatedAt;
@@ -14,13 +14,15 @@ namespace Musify.Infrastructure.Tests.Persistence
        {
            await using var database = fixture.CreateDatabase();

-            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = "roundtrip-user", NormalizedName = "ROUNDTRIP-USER" };
+            var userName = "roundtrip-user";
+            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = userName, NormalizedName = userName.ToUpperInvariant() };
            await database.Users.AddAsync(user, TestContext.Current.CancellationToken);

+            var trackTitle = "Round Trip Track";
            var track = new LocalTrack
            {
-                Title = "Round Trip Track",
-                NormalizedTitle = "ROUND TRIP TRACK",
+                Title = trackTitle,
+                NormalizedTitle = trackTitle.ToUpperInvariant(),
                DurationSeconds = 123.4,
                OwnerUserId = user.Id,
                Owner = user,
@@ -34,7 +36,7 @@ namespace Musify.Infrastructure.Tests.Persistence
            var stored = await reloaded.Tracks.AsNoTracking().SingleAsync(t => t.Id == track.Id, TestContext.Current.CancellationToken);

            Assert.IsType<LocalTrack>(stored);
-            Assert.Equal("Round Trip Track", stored.Title);
+            Assert.Equal(trackTitle, stored.Title);
            Assert.Equal("cover.webp", stored.Pictures.OriginalName);
            Assert.Equal("song.mp3", stored.Audio.OriginalName);
            Assert.Equal(user.Id, ((LocalTrack)stored).OwnerUserId);
@@ -60,8 +62,10 @@ namespace Musify.Infrastructure.Tests.Persistence
        {
            await using var database = fixture.CreateDatabase();

-            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = "cascade-user", NormalizedName = "CASCADE-USER" };
-            var album = new UserAlbum { Title = "Cascade Album", NormalizedTitle = "CASCADE ALBUM", OwnerUserId = user.Id };
+            var userName = "cascade-user";
+            var user = new User { Id = Random.Shared.NextInt64(1, long.MaxValue), Name = userName, NormalizedName = userName.ToUpperInvariant() };
+            var albumTitle = "Cascade Album";
+            var album = new UserAlbum { Title = albumTitle, NormalizedTitle = albumTitle.ToUpperInvariant(), OwnerUserId = user.Id };
            var track = BuildExternalTrack($"cascade-{Guid.NewGuid():N}");
            var link = new AlbumHasTrack { AlbumId = album.Id, TrackId = track.Id, TrackNumber = 1 };

@@ -78,10 +82,13 @@ namespace Musify.Infrastructure.Tests.Persistence
            Assert.True(await verifier.ExternalTracks.AnyAsync(t => t.Id == track.Id, TestContext.Current.CancellationToken));
        }

-        private static ExternalTrack BuildExternalTrack(string externalId) => new()
+        private static ExternalTrack BuildExternalTrack(string externalId)
        {
-            Title = "External",
-            NormalizedTitle = "EXTERNAL",
+            var title = "External";
+            return new ExternalTrack
+            {
+                Title = title,
+                NormalizedTitle = title.ToUpperInvariant(),
                DurationSeconds = 100,
                Source = TrackSource.YouTube,
                ExternalId = externalId,
@@ -90,3 +97,4 @@ namespace Musify.Infrastructure.Tests.Persistence
            };
        }
    }
+}
@@ -131,15 +131,9 @@ namespace Musify.Infrastructure.Tests.Services
            using var httpClient = new HttpClient();
            using var request = new HttpRequestMessage(HttpMethod.Put, uploadUrl)
            {
-                // Not StringContent: it appends "; charset=utf-8" to Content-Type, which no longer
-                // matches the "text/plain" the presigned URL's signature was computed over (SigV4
-                // signs Content-Type byte-for-byte) and the server answers 403 SignatureDoesNotMatch.
                Content = new ByteArrayContent(Encoding.UTF8.GetBytes("presigned content"))
            };
            request.Content.Headers.ContentType = new MediaTypeHeaderValue("text/plain");
-            // GetUploadUrlAsync defaults preventOverwrite to true, which signs the URL together with
-            // this header (see StorageService.GetUploadUrlAsync) — the actual PUT has to send it too,
-            // or the signature no longer matches and the server answers 403.
            request.Headers.TryAddWithoutValidation("If-None-Match", "*");
            var response = await httpClient.SendAsync(request, TestContext.Current.CancellationToken);

@@ -12,13 +12,6 @@ using Xunit;

 namespace Musify.Infrastructure.Tests.TestSupport
 {
-    /// <summary>
-    /// One Postgres container and one SeaweedFS container (same image and startup
-    /// command as <c>deploy/compose.yml</c>), started once for every test in the
-    /// <see cref="InfrastructureCollection"/> and torn down when the run finishes.
-    /// EF Core migrations run once here too, against the real <c>Database</c>
-    /// DbContext, so a broken migration fails the whole run immediately.
-    /// </summary>
    public sealed class InfrastructureTestFixture : IAsyncLifetime
    {
        public const string S3AccessKey = "musify-test";
@@ -51,7 +44,6 @@ namespace Musify.Infrastructure.Tests.TestSupport
            await using var database = new Database(new DatabaseConfiguration { ConnectionString = ConnectionString });
            await database.Database.MigrateAsync();

-            // SeaweedFS does not auto-create buckets (see deploy/compose.yml's seaweedfs-init).
            using var s3 = CreateS3Client();
            await s3.PutBucketAsync(S3Bucket);
        }
@@ -62,9 +54,6 @@ namespace Musify.Infrastructure.Tests.TestSupport
            await seaweedFs.DisposeAsync();
        }

-        /// <summary>A fresh, DI-wired <see cref="Database"/>, so <c>AuditableEntityInterceptor</c>
-        /// (attached in <c>AddDatabase</c>'s DI registration, not in the class itself) is active —
-        /// same wiring the real app uses, not a bare <c>new Database(...)</c>.</summary>
        public Database CreateDatabase()
        {
            var configuration = new ConfigurationBuilder()
@@ -76,8 +65,6 @@ namespace Musify.Infrastructure.Tests.TestSupport
            return services.BuildServiceProvider().GetRequiredService<Database>();
        }

-        /// <summary>An <see cref="IAmazonS3"/> client configured exactly like
-        /// <c>ServicesDependencyInjection.AddStorageService</c>, pointed at the SeaweedFS container.</summary>
        public IAmazonS3 CreateS3Client() =>
            new AmazonS3Client(S3AccessKey, S3SecretKey, new AmazonS3Config
            {