From ec4f99c92ef9d530074477edeea7e2b5c1e1ca52 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 21:08:12 +0900 Subject: [PATCH] Clean up tests some more --- .../Visual/TestCaseBeatmapCarousel.cs | 80 ++++++++++++++----- osu.Game/Screens/Select/BeatmapCarousel.cs | 36 ++++----- 2 files changed, 76 insertions(+), 40 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index f0a0e3405a..da5865c96d 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -9,7 +9,6 @@ using System.Text; using osu.Framework.Allocation; using osu.Framework.Extensions; using osu.Framework.Graphics; -using osu.Framework.MathUtils; using osu.Game.Beatmaps; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; @@ -18,7 +17,7 @@ namespace osu.Game.Tests.Visual { internal class TestCaseBeatmapCarousel : OsuTestCase { - private BeatmapCarousel carousel; + private TestBeatmapCarousel carousel; public override IReadOnlyList RequiredTypes => new[] { @@ -38,7 +37,7 @@ namespace osu.Game.Tests.Visual [BackgroundDependencyLoader] private void load() { - Add(carousel = new BeatmapCarousel + Add(carousel = new TestBeatmapCarousel { RelativeSizeAxes = Axes.Both, }); @@ -47,36 +46,66 @@ namespace osu.Game.Tests.Visual { carousel.Beatmaps = new[] { - createTestBeatmapSet(0), createTestBeatmapSet(1), createTestBeatmapSet(2), createTestBeatmapSet(3), + createTestBeatmapSet(4), }; }); + void checkSelected(int set, int diff) => + AddAssert($"selected is set{set} diff{diff}", () => + carousel.SelectedBeatmap == carousel.Beatmaps.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First()); + + void advanceSelection(bool diff, int direction = 1, int count = 1) + { + if (count == 1) + AddStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => + carousel.SelectNext(direction, !diff)); + else + { + AddRepeatStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => + carousel.SelectNext(direction, !diff), count); + } + } + + void checkVisibleItemCount(bool diff, int count) => + AddAssert($"{count} {(diff ? "diff" : "set")} visible", () => + carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count); + AddUntilStep(() => carousel.Beatmaps.Any(), "Wait for load"); - AddStep("SelectNext set", () => carousel.SelectNext()); - AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + advanceSelection(direction: 1, diff: false); + checkSelected(1, 1); - AddStep("SelectNext diff", () => carousel.SelectNext(1, false)); - AddAssert("set1 diff2", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.Skip(1).First()); + advanceSelection(direction: 1, diff: true); + checkSelected(1, 2); - AddStep("SelectNext backwards", () => carousel.SelectNext(-1)); - AddAssert("set4 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.First()); + advanceSelection(direction: -1, diff: false); + checkSelected(4, 1); - AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); - AddAssert("set3 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Reverse().Skip(1).First().Beatmaps.Last()); + advanceSelection(direction: -1, diff: true); + checkSelected(3, 3); - AddStep("SelectNext", () => carousel.SelectNext()); - AddStep("SelectNext", () => carousel.SelectNext()); - AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + advanceSelection(diff: false); + advanceSelection(diff: false); + checkSelected(1, 1); - AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); - AddAssert("set4 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.Last()); + advanceSelection(direction: -1, diff: true); + checkSelected(4, 3); - // AddStep("Clear beatmaps", () => carousel.Beatmaps = new BeatmapSetInfo[] { }); - // AddStep("SelectNext (noop)", () => carousel.SelectNext()); + AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3" }, false)); + checkVisibleItemCount(diff: false, count: 1); + checkVisibleItemCount(diff: true, count: 3); + checkSelected(3, 1); + + advanceSelection(diff: true, count: 4); + checkSelected(3, 2); + + AddStep("Un-filter (debounce)", () => carousel.Filter(new FilterCriteria())); + AddUntilStep(() => !carousel.PendingFilterTask, "Wait for debounce"); + checkVisibleItemCount(diff: false, count: 4); + checkVisibleItemCount(diff: true, count: 3); } private BeatmapSetInfo createTestBeatmapSet(int i) @@ -89,9 +118,9 @@ namespace osu.Game.Tests.Visual { OnlineBeatmapSetID = 1234 + i, // Create random metadata, then we can check if sorting works based on these - Artist = "MONACA " + RNG.Next(0, 9), - Title = "Black Song " + RNG.Next(0, 9), - AuthorString = "Some Guy " + RNG.Next(0, 9), + Artist = "peppy", + Title = "test set #" + i, + AuthorString = "peppy" }, Beatmaps = new List(new[] { @@ -128,5 +157,12 @@ namespace osu.Game.Tests.Visual }), }; } + + private class TestBeatmapCarousel : BeatmapCarousel + { + public new List Items => base.Items; + + public bool PendingFilterTask => FilterTask != null; + } } } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 970aba0a6d..9f210e023b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -51,7 +51,7 @@ namespace osu.Game.Screens.Select Schedule(() => { scrollableContent.Clear(false); - items.Clear(); + Items.Clear(); carouselSets.Clear(); yPositionsCache.Invalidate(); }); @@ -69,7 +69,7 @@ namespace osu.Game.Screens.Select carouselSets.AddRange(newSets); root = new CarouselGroup(newSets.OfType().ToList()); - items = root.Drawables.Value.ToList(); + Items = root.Drawables.Value.ToList(); yPositionsCache.Invalidate(); BeatmapsChanged?.Invoke(); @@ -88,7 +88,7 @@ namespace osu.Game.Screens.Select private Bindable randomSelectAlgorithm; private readonly List seenSets = new List(); - private List items = new List(); + protected List Items = new List(); private CarouselGroup root = new CarouselGroup(); private readonly Stack randomSelectedBeatmaps = new Stack(); @@ -192,15 +192,15 @@ namespace osu.Game.Screens.Select return; } - int originalIndex = items.IndexOf(selectedBeatmap?.Drawables.Value.First()); + int originalIndex = Items.IndexOf(selectedBeatmap?.Drawables.Value.First()); int currentIndex = originalIndex; // local function to increment the index in the required direction, wrapping over extremities. - int incrementIndex() => currentIndex = (currentIndex + direction + items.Count) % items.Count; + int incrementIndex() => currentIndex = (currentIndex + direction + Items.Count) % Items.Count; while (incrementIndex() != originalIndex) { - var item = items[currentIndex].Item; + var item = Items[currentIndex].Item; if (item.Filtered || item.State == CarouselItemState.Selected) continue; @@ -272,13 +272,13 @@ namespace osu.Game.Screens.Select private FilterCriteria criteria = new FilterCriteria(); - private ScheduledDelegate filterTask; + protected ScheduledDelegate FilterTask; public bool AllowSelection = true; public void FlushPendingFilters() { - if (filterTask?.Completed == false) + if (FilterTask?.Completed == false) Filter(null, false); } @@ -289,7 +289,7 @@ namespace osu.Game.Screens.Select Action perform = delegate { - filterTask = null; + FilterTask = null; carouselSets.ForEach(s => s.Filter(criteria)); @@ -301,11 +301,11 @@ namespace osu.Game.Screens.Select select(selectedBeatmap); }; - filterTask?.Cancel(); - filterTask = null; + FilterTask?.Cancel(); + FilterTask = null; if (debounce) - filterTask = Scheduler.AddDelayed(perform, 250); + FilterTask = Scheduler.AddDelayed(perform, 250); else perform(); } @@ -362,7 +362,7 @@ namespace osu.Game.Screens.Select foreach (var d in set.Drawables.Value) { - items.Remove(d); + Items.Remove(d); scrollableContent.Remove(d); } @@ -385,7 +385,7 @@ namespace osu.Game.Screens.Select float lastSetY = 0; - foreach (DrawableCarouselItem d in items) + foreach (DrawableCarouselItem d in Items) { switch (d) { @@ -474,7 +474,7 @@ namespace osu.Game.Screens.Select }); // Find index range of all items that should be on-screen - Trace.Assert(items.Count == yPositions.Count); + Trace.Assert(Items.Count == yPositions.Count); int firstIndex = yPositions.BinarySearch(Current - DrawableCarouselItem.MAX_HEIGHT); if (firstIndex < 0) firstIndex = ~firstIndex; @@ -484,21 +484,21 @@ namespace osu.Game.Screens.Select lastIndex = ~lastIndex; // Add the first item of the last visible beatmap group to preload its data. - if (lastIndex != 0 && items[lastIndex - 1] is DrawableCarouselBeatmapSet) + if (lastIndex != 0 && Items[lastIndex - 1] is DrawableCarouselBeatmapSet) lastIndex++; } // Add those items within the previously found index range that should be displayed. for (int i = firstIndex; i < lastIndex; ++i) { - DrawableCarouselItem item = items[i]; + DrawableCarouselItem item = Items[i]; // Only add if we're not already part of the content. if (!scrollableContent.Contains(item)) { // Makes sure headers are always _below_ items, // and depth flows downward. - item.Depth = i + (item is DrawableCarouselBeatmapSet ? items.Count : 0); + item.Depth = i + (item is DrawableCarouselBeatmapSet ? Items.Count : 0); switch (item.LoadState) {