From 1906c2f72537fde9386f03313afa3e95ba9b1663 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 18 Jul 2024 15:57:57 +0900 Subject: [PATCH 1/8] Fix TestTouchScreenDetectionAtSongSelect test failure https://github.com/ppy/osu/actions/runs/9985890747/job/27597501295 In this case, the settings overlay is taking a very long time to load (on a background thread), and pops in when it finishes loading because it's been requested to open. The opens the settings overlay, closes it (by pressing escape, this does not actually close it because it's not loaded yet), and then enters song select by pressing 'P' 3 times. The settings overlay finishes loading at just the right opportune moment to eat one of the 'P' key presses. --- osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index e81c6d2e86..3ae1d9786d 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -952,6 +952,8 @@ namespace osu.Game.Tests.Visual.Navigation [Test] public void TestTouchScreenDetectionAtSongSelect() { + AddUntilStep("wait for settings", () => Game.Settings.IsLoaded); + AddStep("touch logo", () => { var button = Game.ChildrenOfType().Single(); From 7bb680a8a445cd7a87ea206627f925a08dd0a337 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 18 Jul 2024 16:01:09 +0900 Subject: [PATCH 2/8] Raise workflow timeout time https://github.com/ppy/osu/actions/runs/9985890747/job/27597500883 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ea4654563..dc1cb6c186 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,7 +67,7 @@ jobs: - { prettyname: macOS, fullname: macos-latest } - { prettyname: Linux, fullname: ubuntu-latest } threadingMode: ['SingleThread', 'MultiThreaded'] - timeout-minutes: 60 + timeout-minutes: 120 steps: - name: Checkout uses: actions/checkout@v4 From f3cd3d7d3b8c7d56746b6d93a103d71ef6de991e Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 18 Jul 2024 16:22:39 +0900 Subject: [PATCH 3/8] Fix TestAllSamplesStopDuringSeek test failure https://github.com/smoogipoo/osu/actions/runs/9986761756/job/27599851263 This is a bit of a workaround, likely timing related. I don't foresee an until step in this case to cause false-passes. --- .../Visual/Gameplay/TestSceneGameplaySamplePlayback.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs index ad3fe7cb7e..21c83d521c 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Visual.Gameplay return true; }); - AddAssert("sample playback disabled", () => sampleDisabler.SamplePlaybackDisabled.Value); + AddUntilStep("sample playback disabled", () => sampleDisabler.SamplePlaybackDisabled.Value); // because we are in frame stable context, it's quite likely that not all samples are "played" at this point. // the important thing is that at least one started, and that sample has since stopped. From 33a81d818107b80c73c4dd7ad483a9c59abb474a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 18 Jul 2024 18:34:08 +0900 Subject: [PATCH 4/8] Use constraint to improve assertion message --- .../Visual/Navigation/TestSceneBeatmapEditorNavigation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs index b5dfa9a87f..99d1ff93c5 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs @@ -209,7 +209,7 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("Close editor while loading", () => Game.ScreenStack.CurrentScreen.Exit()); AddUntilStep("Wait for menu", () => Game.ScreenStack.CurrentScreen is MainMenu); - AddAssert("Check no new beatmaps were made", () => allBeatmapSets().SequenceEqual(beatmapSets)); + AddAssert("Check no new beatmaps were made", allBeatmapSets, () => Is.EquivalentTo(beatmapSets)); BeatmapSetInfo[] allBeatmapSets() => Game.Realm.Run(realm => realm.All().Where(x => !x.DeletePending).ToArray()); } From 3f4e56be3ce3532b6c7b75f530a1a2b7c45a99ca Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 18 Jul 2024 20:53:59 +0900 Subject: [PATCH 5/8] Fix TestPostAsOwner test failure https://github.com/smoogipoo/osu/actions/runs/9990112749/job/27610257309 Comments are loaded asynchronously, both from the initial request and the following message-post request. By sheer timing luck, these could be out of order and the assertion on the posted message could fail. --- osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs index fd3552f675..acc3c9b8b4 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneCommentsContainer.cs @@ -157,6 +157,7 @@ namespace osu.Game.Tests.Visual.Online { setUpCommentsResponse(getExampleComments()); AddStep("show comments", () => commentsContainer.ShowComments(CommentableType.Beatmapset, 123)); + AddUntilStep("comments shown", () => commentsContainer.ChildrenOfType().Any()); setUpPostResponse(); AddStep("enter text", () => editorTextBox.Current.Value = "comm"); @@ -175,6 +176,7 @@ namespace osu.Game.Tests.Visual.Online { setUpCommentsResponse(getExampleComments()); AddStep("show comments", () => commentsContainer.ShowComments(CommentableType.Beatmapset, 123)); + AddUntilStep("comments shown", () => commentsContainer.ChildrenOfType().Any()); setUpPostResponse(true); AddStep("enter text", () => editorTextBox.Current.Value = "comm"); From d7ae9505b2c3fe826d769c64279468671de94d82 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 19 Jul 2024 14:08:05 +0900 Subject: [PATCH 6/8] Fix TestCancelNavigationToEditor test failure https://github.com/ppy/osu/actions/runs/10002179087/job/27648253709 The editor could be pushed before the exit actually occurs. --- .../TestSceneBeatmapEditorNavigation.cs | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs index 99d1ff93c5..5640682d06 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs @@ -1,12 +1,16 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.IO; using System.Linq; +using System.Threading; using NUnit.Framework; +using osu.Framework.Allocation; using osu.Framework.Extensions; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Extensions.ObjectExtensions; +using osu.Framework.Graphics; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input; using osu.Framework.Screens; @@ -204,9 +208,13 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("Set current beatmap to default", () => Game.Beatmap.SetDefault()); - AddStep("Push editor loader", () => Game.ScreenStack.Push(new EditorLoader())); + DelayedLoadEditorLoader loader = null!; + AddStep("Push editor loader", () => Game.ScreenStack.Push(loader = new DelayedLoadEditorLoader())); AddUntilStep("Wait for loader current", () => Game.ScreenStack.CurrentScreen is EditorLoader); + AddUntilStep("wait for editor load start", () => loader.Editor != null); AddStep("Close editor while loading", () => Game.ScreenStack.CurrentScreen.Exit()); + AddStep("allow editor load", () => loader.AllowLoad.Set()); + AddUntilStep("wait for editor ready", () => loader.Editor!.LoadState >= LoadState.Ready); AddUntilStep("Wait for menu", () => Game.ScreenStack.CurrentScreen is MainMenu); AddAssert("Check no new beatmaps were made", allBeatmapSets, () => Is.EquivalentTo(beatmapSets)); @@ -356,5 +364,33 @@ namespace osu.Game.Tests.Visual.Navigation private EditorBeatmap getEditorBeatmap() => getEditor().ChildrenOfType().Single(); private Editor getEditor() => (Editor)Game.ScreenStack.CurrentScreen; + + private partial class DelayedLoadEditorLoader : EditorLoader + { + public readonly ManualResetEventSlim AllowLoad = new ManualResetEventSlim(); + public Editor? Editor { get; private set; } + + protected override Editor CreateEditor() => Editor = new DelayedLoadEditor(this); + } + + private partial class DelayedLoadEditor : Editor + { + private readonly DelayedLoadEditorLoader loader; + + public DelayedLoadEditor(DelayedLoadEditorLoader loader) + : base(loader) + { + this.loader = loader; + } + + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) + { + // Importantly, this occurs before base.load(). + if (!loader.AllowLoad.Wait(TimeSpan.FromSeconds(10))) + throw new TimeoutException(); + + return base.CreateChildDependencies(parent); + } + } } } From 7a4758d8ccbf10e8118f4e71ff0075b4107b3c4d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 19 Jul 2024 17:49:13 +0900 Subject: [PATCH 7/8] Attempt to fix TestSelectableMouseHandling test failure https://github.com/ppy/osu/pull/28900/checks?check_run_id=27652166871 This is an attempt. Going frame-by-frame I noticed that there's one frame in which the text is loaded but the FillFlowContainer/GridContainer haven't properly validated so the text is not positioned correctly (it's overflowing the panel to the left). If the cursor is moved at this exact time, then it may not be properly positioned for the following assertion, even though it is _somewhere_ on the panel. If the above is the case, then this is a known o!f issue, but not a simple one to solve. I haven't reproed this locally. --- .../TestSceneDrawableRoomPlaylist.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index bd62a8b131..2ef56bd54e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -12,6 +12,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Sprites; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Game.Beatmaps; @@ -319,16 +320,17 @@ namespace osu.Game.Tests.Visual.Multiplayer }); AddUntilStep("wait for load", () => playlist.ChildrenOfType().Any() && playlist.ChildrenOfType().First().DrawWidth > 0); - AddStep("move mouse to first item title", () => - { - var drawQuad = playlist.ChildrenOfType().First().ScreenSpaceDrawQuad; - var location = (drawQuad.TopLeft + drawQuad.BottomLeft) / 2 + new Vector2(drawQuad.Width * 0.2f, 0); - InputManager.MoveMouseTo(location); - }); + + AddStep("move mouse to first item title", () => InputManager.MoveMouseTo(playlist.ChildrenOfType().First().ChildrenOfType().First())); AddAssert("first item title not hovered", () => playlist.ChildrenOfType().First().IsHovered, () => Is.False); - AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + + AddStep("click title", () => + { + InputManager.MoveMouseTo(playlist.ChildrenOfType().First().ChildrenOfType().First()); + InputManager.Click(MouseButton.Left); + }); + AddUntilStep("first item selected", () => playlist.ChildrenOfType().First().IsSelectedItem, () => Is.True); - // implies being clickable. AddUntilStep("first item title hovered", () => playlist.ChildrenOfType().First().IsHovered, () => Is.True); AddStep("move mouse to second item results button", () => InputManager.MoveMouseTo(playlist.ChildrenOfType().ElementAt(5))); From 5af39aad00878d577fb57fcd020ffd8340cfffe9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 19 Jul 2024 19:02:41 +0900 Subject: [PATCH 8/8] Add beatmap name to log string Makes it easy to compare this line versus the one in OsuGame.PresentBeatmap(). At the moment it's just GUID which is... not useful! --- osu.Game/Screens/Select/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 14c4a34d14..307043a312 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -609,7 +609,7 @@ namespace osu.Game.Screens.Select // clear pending task immediately to track any potential nested debounce operation. selectionChangedDebounce = null; - Logger.Log($"Song select updating selection with beatmap:{beatmap?.ID.ToString() ?? "null"} ruleset:{ruleset?.ShortName ?? "null"}"); + Logger.Log($"Song select updating selection with beatmap: {beatmap} {beatmap?.ID.ToString() ?? "null"} ruleset:{ruleset?.ShortName ?? "null"}"); if (transferRulesetValue()) {