From 000cb7572651658661f8f26ee9057c12ace65e98 Mon Sep 17 00:00:00 2001 From: Jason Dove <1695733+jasongdove@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:43:37 -0500 Subject: [PATCH] cleanup --- .../AlternateScheduleAnchorTests.cs | 19 ++------ .../ClassicScheduling/ContinuePlayoutTests.cs | 3 +- .../ClassicScheduling/MultiDayBuildTests.cs | 46 ++++++------------- .../Scheduling/MarathonEnumeratorTests.cs | 29 +++--------- .../Scheduling/PlaylistEnumeratorTests.cs | 12 ++--- .../Scheduling/ShuffledContentTests.cs | 7 +-- .../Scheduling/PlaylistEnumerator.cs | 27 +++-------- ErsatzTV.Core/Scheduling/PlayoutBuilder.cs | 28 ++++------- .../SeasonEpisodeMediaCollectionEnumerator.cs | 5 +- 9 files changed, 50 insertions(+), 126 deletions(-) diff --git a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/AlternateScheduleAnchorTests.cs b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/AlternateScheduleAnchorTests.cs index df6513926..6091bce38 100644 --- a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/AlternateScheduleAnchorTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/AlternateScheduleAnchorTests.cs @@ -11,24 +11,16 @@ using Testably.Abstractions.Testing; namespace ErsatzTV.Core.Tests.Scheduling.ClassicScheduling; -/// -/// A collection used only by an alternate schedule still gets an enumerator on days when that -/// alternate isn't active, because the collection media items are gathered once for the primary -/// schedule and every alternate. On those days it falls into the filler branch and gets a -/// shuffled enumerator, whose index domain is the grouped item count rather than the media item -/// count. -/// +// a collection used only by an alternate schedule still gets an enumerator on days that alternate +// isn't active, because media items are gathered once for the primary schedule and every alternate [TestFixture] public class AlternateScheduleAnchorTests : PlayoutBuilderTestBase { private const int PrimaryCollectionId = 1; private const int AlternateCollectionId = 2; - /// - /// The alternate's collection has 10 episodes and its schedule orders them by season/episode, - /// so its own enumerator's index domain is 0-9. With a multi-part pair the shuffled enumerator - /// used on inactive days groups those 10 items into 9, so index 9 is out of its domain. - /// + // season/episode order over 10 episodes indexes 0-9; with a multi-part pair, the shuffled + // enumerator used on inactive days sees 9 groups, so index 9 is outside its domain [TestCase(9, true, ExpectedResult = 9, TestName = "Index at the grouped count")] [TestCase(8, true, ExpectedResult = 8, TestName = "Index below the grouped count")] [TestCase(9, false, ExpectedResult = 9, TestName = "Index at the grouped count, nothing grouped")] @@ -47,8 +39,7 @@ public class AlternateScheduleAnchorTests : PlayoutBuilderTestBase EnumeratorState = new CollectionEnumeratorState { Seed = 1234, Index = index } }); - // a Wednesday, so the alternate (Sundays only) is not active and the alternate's - // collection is not scheduled at all + // a Wednesday, so the alternate (Sundays only) is not active DateTimeOffset now = LocalTime(new DateTime(2025, 6, 11), 20); now.DayOfWeek.ShouldBe(DayOfWeek.Wednesday); diff --git a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/ContinuePlayoutTests.cs b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/ContinuePlayoutTests.cs index 62c961230..2586e3a6d 100644 --- a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/ContinuePlayoutTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/ContinuePlayoutTests.cs @@ -468,8 +468,7 @@ public class ContinuePlayoutTests : PlayoutBuilderTestBase result.AddedItems.Count.ShouldBe(53); } - // two collections, each with a checkpoint for the two midnights the build crossed - // plus a continue anchor + // two collections, each with a continue anchor and a checkpoint per midnight crossed playout.ProgramScheduleAnchors.Count.ShouldBe(6); playout.ProgramScheduleAnchors.Count(x => x.AnchorDate is not null).ShouldBe(4); PlayoutProgramScheduleAnchor lastCheckpoint = playout.ProgramScheduleAnchors diff --git a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/MultiDayBuildTests.cs b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/MultiDayBuildTests.cs index b7142c411..efbc73ad5 100644 --- a/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/MultiDayBuildTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/MultiDayBuildTests.cs @@ -11,11 +11,8 @@ using Testably.Abstractions.Testing; namespace ErsatzTV.Core.Tests.Scheduling.ClassicScheduling; -/// -/// Unit-test version of the "fast forward a playout" debugging endpoint that used to live in -/// ChannelController: run hourly continue builds across several days and assert invariants at -/// every step. -/// +// unit-test version of the "fast forward a playout" debugging endpoint that used to live in +// ChannelController [TestFixture] public class MultiDayBuildTests : PlayoutBuilderTestBase { @@ -25,13 +22,11 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase [Test] public async Task Continue_Should_Keep_A_Checkpoint_For_The_Current_Day() { - // 6 hour movies scheduled 3 at a time, so a block spans 18 hours and regularly - // crosses midnight - which is expected and intended + // 6 hour movies 3 at a time, so an 18 hour block regularly crosses midnight (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData( TimeSpan.FromHours(6), multipleCount: "3"); - // a playout created at 20:00, then built every hour for a week DateTimeOffset start = LocalTime(20); var missing = new List(); @@ -48,15 +43,13 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase result.IsRight.ShouldBeTrue($"build failed at {now:yyyy-MM-dd HH:mm}"); - // a build never writes a checkpoint for its own first day, and doesn't need to: - // there is no earlier state to restore, so refreshing starts from index 0 and - // reproduces the same schedule + // no checkpoint is written for a build's own first day, and none is needed: there is no + // earlier state, so refreshing from index 0 reproduces the same schedule if (now.Date == start.Date) { continue; } - // from here on, a refresh has to have somewhere to rewind to bool hasCheckpointForToday = playout.ProgramScheduleAnchors.Any(a => a.AnchorDateOffset.HasValue && a.AnchorDateOffset.Value.Date == now.Date); @@ -74,8 +67,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase [Test] public async Task Refresh_Should_Not_Reset_Collection_Progress() { - // a collection large enough that the index can't wrap over the whole run, so the - // index is a straightforward measure of how much progress has been made + // large enough that the index can't wrap, so it measures progress directly (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData( TimeSpan.FromHours(6), multipleCount: "3", @@ -83,8 +75,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase DateTimeOffset start = LocalTime(20); - // three weeks of hourly builds, so that progress is far enough along to be clearly - // distinguishable from a playout that restarted at the beginning + // long enough that progress is clearly distinguishable from a playout that restarted for (var hour = 0; hour < 24 * 21; hour++) { await builder.Build( @@ -107,9 +98,8 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase result.IsRight.ShouldBeTrue(); - // a refresh rewinds to the start of today, so the index legitimately moves back by - // up to a day. a refresh that lost its checkpoints restarts at 0 and only advances - // through the build window, landing an order of magnitude lower + // rewinding to the start of today legitimately moves the index back by up to a day; a refresh + // that lost its checkpoints lands an order of magnitude lower int after = HeadIndex(playout); after.ShouldBeGreaterThan( before / 2, @@ -119,8 +109,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase [Test] public async Task Continue_Should_Keep_A_Checkpoint_For_Every_Day_In_The_Build_Window() { - // shaped like the playouts that were missing checkpoints in a real user database: a week - // long build window, shuffled schedule items, and blocks of five ~100 minute items + // shaped like the playouts that were missing checkpoints in a real user database (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData( TimeSpan.FromMinutes(100), multipleCount: "5", @@ -145,9 +134,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase result.IsRight.ShouldBeTrue($"build failed at {now:yyyy-MM-dd HH:mm}"); - // every day the playout covers needs a checkpoint, so that a refresh on any of them has - // somewhere to rewind to. the build's own first day is the one exception, and days - // already in the past are pruned + // every day covered needs a checkpoint, except the build's own first day; past days are pruned DateTime firstExpected = now.Date > start.Date ? now.Date : start.Date.AddDays(1); DateTime lastExpected = now.AddDays(WeekOfDaysToBuild).Date; @@ -263,14 +250,9 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase return (builder, playout, referenceData); } - /// - /// Anchor dates are compared in machine-local time - /// (), so the test clock has to be local - /// too, or day boundaries won't line up. That means these tests run in whatever zone the machine is - /// in - UTC on CI, something else locally - so the window below is deliberately placed in mid-June, - /// where no zone has a DST transition. Day-boundary behaviour across a DST change is a separate - /// concern and wants its own test. - /// + // anchor dates are compared in machine-local time, so the test clock has to be local too or day + // boundaries won't line up. that means running in whatever zone the machine is in, hence mid-June, + // where no zone has a DST transition private static DateTimeOffset LocalTime(int hour) { var date = new DateTime(2025, 6, 10, 0, 0, 0, DateTimeKind.Unspecified); diff --git a/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs b/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs index e8be029ce..7c9d30358 100644 --- a/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs @@ -8,11 +8,8 @@ using Shouldly; namespace ErsatzTV.Core.Tests.Scheduling; -/// -/// Marathon content builds a over one playlist item per group, -/// so it inherits that enumerator's save/restore behaviour. These cover the state handling, not -/// the grouping. -/// +// marathon content builds a PlaylistEnumerator over one playlist item per group, so it inherits that +// enumerator's save/restore behaviour; these cover the state handling, not the grouping [TestFixture] public class MarathonEnumeratorTests { @@ -23,11 +20,7 @@ public class MarathonEnumeratorTests private CancellationToken _cancellationToken; - /// - /// A cycle completes when every media item has been played, which resets the index to 0 and - /// reshuffles the groups. Two shows of three episodes each is six items, so the index has to - /// come back to 0 within six moves. - /// + // a cycle completes when every media item has been played, which resets the index to 0 [Test] public async Task Marathon_Should_Complete_A_Cycle() { @@ -44,10 +37,8 @@ public class MarathonEnumeratorTests indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed"); } - /// - /// Season 0 episodes are filtered out by the season/episode enumerator but still counted as - /// items the cycle is waiting on, so the cycle can never complete. - /// + // season 0 is filtered out by the season/episode enumerator but still counted as an item the cycle + // is waiting on, so the cycle can never complete [Test] public async Task Marathon_Should_Complete_A_Cycle_With_Specials() { @@ -65,10 +56,8 @@ public class MarathonEnumeratorTests indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed"); } - /// - /// The flood and duration schedulers snapshot every enumerator's state before trying an item - /// and hand it back when the item doesn't fit, so ResetState has to actually rewind. - /// + // the flood and duration schedulers hand back a snapshot when an item doesn't fit, so ResetState + // has to actually rewind [Test] public async Task Marathon_Should_Rewind_On_ResetState() { @@ -94,10 +83,6 @@ public class MarathonEnumeratorTests enumerator.Current.Map(mi => mi.Id).IfNone(-1).ShouldBe(expected); } - /// - /// Rebuilding a playout restores the enumerator from the persisted state, which has to land on - /// the same item the previous build stopped at. - /// [Test] public async Task Marathon_Should_Resume_Where_It_Left_Off() { diff --git a/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs b/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs index 2a75ba8d7..369733a37 100644 --- a/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs @@ -457,18 +457,14 @@ public class PlaylistEnumeratorTests continued.ShouldBe(reference.Skip(6).Take(6)); } - /// - /// Completing a cycle reshuffles the playlist items, and the state persisted at that point is only a - /// seed and an index. Rebuilding starts from the playlist order and applies the seed, so the shuffle - /// has to be a function of those two things - otherwise the rebuilt playout diverges from the one - /// the previous build was part way through. - /// + // a rebuild only has the persisted seed and index, so a shuffle that depends on anything else + // diverges from the playout the previous build was part way through [Test] public async Task Shuffled_Playlist_Should_Resume_The_Same_Way_After_A_Cycle() { IMediaCollectionRepository repo = Substitute.For(); - // four single-item entries, so a cycle is four moves and each move names its own playlist item + // single-item entries, so each move names its own playlist item Dictionary> BuildMap() => Range(1, 4).ToDictionary( i => new PlaylistItem @@ -493,7 +489,7 @@ public class PlaylistEnumeratorTests randomStartPoint: false, CancellationToken.None); - // play past the end of the first cycle, which reseeds and reshuffles + // past the end of the first cycle, which reseeds and reshuffles for (var i = 0; i < 6; i++) { live.MoveNext(Option.None); diff --git a/ErsatzTV.Core.Tests/Scheduling/ShuffledContentTests.cs b/ErsatzTV.Core.Tests/Scheduling/ShuffledContentTests.cs index d83b5b243..444bf565f 100644 --- a/ErsatzTV.Core.Tests/Scheduling/ShuffledContentTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/ShuffledContentTests.cs @@ -134,11 +134,8 @@ public class ShuffledContentTests shuffledContent.State.Seed.ShouldNotBe(MagicSeed); } - /// - /// MoveNext walks (and wraps at) the flattened item count, so an index up to one less than - /// that is valid state. The constructor validates against the group count instead, so any - /// index at or above it is thrown away even though the enumerator itself produced it. - /// + // MoveNext wraps at the flattened item count, so any index below that is valid state the + // enumerator produced itself [Test] public void State_Should_Not_Reset_When_Valid_For_Grouped_Items() { diff --git a/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs b/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs index 87c97e7fb..e2b8f22d9 100644 --- a/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs +++ b/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs @@ -15,8 +15,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator private bool _shufflePlaylistItems; private List _sortedEnumerators; - // a cycle start is the only position this enumerator can be put back to directly; every other - // position is reached by replaying from one. see RewindToCycleStart + // a cycle start is the only position that can be restored directly; see RewindToCycleStart private List _enumeratorsInPlaylistOrder; private Dictionary _childStatesAtCycleStart; @@ -37,11 +36,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator public int EnumeratorIndex { get; private set; } - /// - /// A playlist position isn't described by index and seed alone - it also lives in - /// , the batch progress, the ids the cycle still owes and the state of - /// every child enumerator - so the only way back to a position is to replay to it. - /// + // index and seed don't describe a playlist position, so the only way back to one is to replay to it public void ResetState(CollectionEnumeratorState state) { // nothing has moved since this state was handed out, so there is nothing to rewind @@ -164,8 +159,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator { List items = playlistItemMap[playlistItem]; - // a cycle ends when every item has been played, so an item that can never be played would - // leave the cycle permanently incomplete; season, episode order won't play season 0 + // an item that can never play would leave the cycle permanently incomplete if (playlistItem.PlaybackOrder is PlaybackOrder.SeasonEpisode) { items = SeasonEpisodeMediaCollectionEnumerator.Playable(items); @@ -226,8 +220,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator } enumerator = new SeasonEpisodeMediaCollectionEnumerator(items, initState); - // every season 0 episode was already removed above, so this is a playlist item - // that had nothing else in it + // season 0 is already gone, so this item had nothing else in it if (enumerator.Count == 0) { enumerator = null; @@ -309,10 +302,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator } } - /// - /// Puts everything back the way leaves it for the given seed: children at their - /// starting positions, the playlist items back in seed order, and the cycle owing every item again. - /// + // must leave exactly what Create leaves for this seed, or replaying from here won't match a rebuild private void RewindToCycleStart(int seed) { foreach ((IMediaCollectionEnumerator enumerator, CollectionEnumeratorState childState) in @@ -358,11 +348,8 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator } } - /// - /// Always shuffles the playlist order rather than the current order, so that the result is a function - /// of that order and the seed - which is all the persisted state gives a rebuild to work from. Cycles - /// reshuffle from the same starting point rather than from wherever the last shuffle landed. - /// + // shuffles playlist order rather than current order, so the result depends only on that order and the + // seed - all a rebuild has to work from private List ShufflePlaylistItems() { if (_enumeratorsInPlaylistOrder.Count < 3) diff --git a/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs b/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs index 10c5c202b..d95f60a09 100644 --- a/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs +++ b/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs @@ -169,9 +169,8 @@ public class PlayoutBuilder : IPlayoutBuilder // _logger.LogDebug("All anchors: {@Anchors}", playout.ProgramScheduleAnchors); - // remove the "continue" anchors (null anchor date), which hold the state at the head of - // the playout; they have to go rather than just be ignored, because GetMediaCollectionEnumerator - // ranks a null anchor date above every checkpoint and we want to rewind to today's checkpoint + // these have to go rather than just be ignored; GetMediaCollectionEnumerator ranks a null + // anchor date above every checkpoint, and we want to rewind to today's checkpoint playout.ProgramScheduleAnchors.RemoveAll(a => a.AnchorDate is null); // _logger.LogDebug("Checkpoint anchors: {@Anchors}", playout.ProgramScheduleAnchors); @@ -457,8 +456,8 @@ public class PlayoutBuilder : IPlayoutBuilder return result; } - // build one whole day at a time; this is also how alternate schedules switch per day, - // since the active schedule is selected once per pass + // one whole day at a time; this is also how alternate schedules switch, since the active + // schedule is selected once per pass while (finish < playoutFinish) { if (cancellationToken.IsCancellationRequested) @@ -497,7 +496,6 @@ public class PlayoutBuilder : IPlayoutBuilder if (start < playoutFinish) { - // build the remaining partial day _logger.LogDebug("Building final playout from {Start} to {Finish}", start, playoutFinish); Either buildResult = await BuildPlayoutItemsForPass( playout, @@ -901,10 +899,8 @@ public class PlayoutBuilder : IPlayoutBuilder result.AddedItems.AddRange(playoutItems); - // save a checkpoint anchor whenever scheduling crosses a local midnight; this has to - // happen here rather than at the end of a pass, because pass boundaries only - // aspirationally line up with midnights - the final partial pass never ends on one, and - // a long item can carry a whole-day pass well past its own boundary + // has to happen here rather than at the end of a pass; pass boundaries only aspirationally + // line up with midnights, and the final partial pass never ends on one if (nextState.CurrentTime.ToLocalTime().Date > playoutBuilderState.CurrentTime.ToLocalTime().Date) { SaveCheckpointAnchors(playout, collectionEnumerators, nextState.CurrentTime); @@ -1212,11 +1208,7 @@ public class PlayoutBuilder : IPlayoutBuilder && anchor.PlaylistId == collectionKey.PlaylistId && anchor.SearchQuery == collectionKey.SearchQuery; - /// - /// Records the collection progress as of , which is the first - /// resumable point at or after a local midnight. Refresh rewinds to the checkpoint dated today, - /// so every local day of the playout needs one. - /// + // refresh rewinds to the checkpoint dated today, so every local day of the playout needs one private static void SaveCheckpointAnchors( Playout playout, Dictionary collectionEnumerators, @@ -1227,15 +1219,13 @@ public class PlayoutBuilder : IPlayoutBuilder foreach ((CollectionKey collectionKey, IMediaCollectionEnumerator enumerator) in collectionEnumerators) { - // one checkpoint per collection key per local date; rebuilding a day that already has a - // checkpoint replaces it, since the new state is the one that matches the new items + // rebuilding a day replaces its checkpoint; the new state is the one matching the new items Option maybeExisting = playout.ProgramScheduleAnchors.FirstOrDefault(a => AnchorMatchesCollectionKey(a, collectionKey) && a.AnchorDateOffset.HasValue && a.AnchorDateOffset.Value.Date == checkpointDate); - // the enumerator keeps mutating its own state as the build continues, so the checkpoint - // needs a copy of it, frozen at this instant + // the enumerator keeps mutating its own state as the build continues CollectionEnumeratorState state = enumerator.State.Clone(); foreach (PlayoutProgramScheduleAnchor existing in maybeExisting) diff --git a/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs b/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs index cff1577fb..eabfe8164 100644 --- a/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs +++ b/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs @@ -34,10 +34,7 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu } } - /// - /// Season 0 is never played in season, episode order. Callers that need to know which items this - /// enumerator will actually play - rather than which items they handed it - share the rule here. - /// + // shared with callers that need to know what this will play, not what they handed it public static List Playable(IEnumerable mediaItems) => mediaItems.Filter(mi => (mi is not Episode episode) || (episode.Season?.SeasonNumber ?? 0) > 0).ToList();