From 607a04ae7319b823da095c948be9897767ff84bd Mon Sep 17 00:00:00 2001
From: Cootz <artem2004799@gmail.com>
Date: Fri, 28 Apr 2023 17:45:00 +0300
Subject: [PATCH 1/7] Fix the issue
---
osu.Game/Screens/Edit/Editor.cs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs
index d89392f757..b5d304a031 100644
--- a/osu.Game/Screens/Edit/Editor.cs
+++ b/osu.Game/Screens/Edit/Editor.cs
@@ -210,7 +210,10 @@ namespace osu.Game.Screens.Edit
// this is a bit haphazard, but guards against setting the lease Beatmap bindable if
// the editor has already been exited.
if (!ValidForPush)
+ {
+ beatmapManager.Delete(loadableBeatmap.BeatmapSetInfo);
return;
+ }
}
try
From c5357d30ab0c545a7eb4dd652eeefcb828c1f433 Mon Sep 17 00:00:00 2001
From: Cootz <artem2004799@gmail.com>
Date: Fri, 28 Apr 2023 20:36:31 +0300
Subject: [PATCH 2/7] Add test
---
.../Navigation/TestSceneBeatmapEditor.cs | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
new file mode 100644
index 0000000000..67835ed0f5
--- /dev/null
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
@@ -0,0 +1,57 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using DeepEqual.Syntax;
+using NUnit.Framework;
+using osu.Framework.Screens;
+using osu.Game.Beatmaps;
+using osu.Game.Screens.Edit;
+using osu.Game.Screens.Menu;
+
+namespace osu.Game.Tests.Visual.Navigation
+{
+ public partial class TestSceneBeatmapEditor : OsuGameTestScene
+ {
+ [Test]
+ public void TestCancelNavigationToEditor()
+ {
+ BeatmapSetInfo[] beatmapSets = Array.Empty<BeatmapSetInfo>();
+
+ AddStep("Timestamp current beatmapsets", () =>
+ {
+ Game.Realm.Run(realm =>
+ {
+ beatmapSets = realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray();
+ });
+ });
+
+ AddStep("Open editor and close it while loading", () =>
+ {
+ var task = Task.Run(async () =>
+ {
+ await Task.Delay(100);
+ Game.ScreenStack.CurrentScreen.Exit();
+ });
+
+ Game.ScreenStack.Push(new EditorLoader());
+ });
+
+ AddUntilStep("wait for editor", () => Game.ScreenStack.CurrentScreen is MainMenu);
+
+ BeatmapSetInfo[] currentSetInfos = Array.Empty<BeatmapSetInfo>();
+
+ AddStep("Get current beatmaps", () =>
+ {
+ Game.Realm.Run(realm =>
+ {
+ currentSetInfos = realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray();
+ });
+ });
+
+ AddAssert("dummy beatmap didn't appear", () => currentSetInfos.IsDeepEqual(beatmapSets));
+ }
+ }
+}
From d9b3c97179d2f9fd5114909fcb323208ff630f22 Mon Sep 17 00:00:00 2001
From: Cootz <artem2004799@gmail.com>
Date: Fri, 28 Apr 2023 21:23:00 +0300
Subject: [PATCH 3/7] Fix testing
---
.../Navigation/TestSceneBeatmapEditor.cs | 26 +++++++++++--------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
index 67835ed0f5..9760fc2c97 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
@@ -1,9 +1,7 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
-using System;
using System.Linq;
-using System.Threading.Tasks;
using DeepEqual.Syntax;
using NUnit.Framework;
using osu.Framework.Screens;
@@ -18,7 +16,7 @@ namespace osu.Game.Tests.Visual.Navigation
[Test]
public void TestCancelNavigationToEditor()
{
- BeatmapSetInfo[] beatmapSets = Array.Empty<BeatmapSetInfo>();
+ BeatmapSetInfo[]? beatmapSets = null;
AddStep("Timestamp current beatmapsets", () =>
{
@@ -28,20 +26,26 @@ namespace osu.Game.Tests.Visual.Navigation
});
});
- AddStep("Open editor and close it while loading", () =>
+ AddStep("Set current beatmap to default", () =>
{
- var task = Task.Run(async () =>
- {
- await Task.Delay(100);
- Game.ScreenStack.CurrentScreen.Exit();
- });
+ Game.Beatmap.SetDefault();
+ });
+ AddStep("Open editor loader", () =>
+ {
Game.ScreenStack.Push(new EditorLoader());
});
+ AddUntilStep("wait for editor", () => Game.ScreenStack.CurrentScreen is EditorLoader);
+
+ AddStep("close editor while loading", () =>
+ {
+ Game.ScreenStack.CurrentScreen.Exit();
+ });
+
AddUntilStep("wait for editor", () => Game.ScreenStack.CurrentScreen is MainMenu);
- BeatmapSetInfo[] currentSetInfos = Array.Empty<BeatmapSetInfo>();
+ BeatmapSetInfo[]? currentSetInfos = null;
AddStep("Get current beatmaps", () =>
{
@@ -51,7 +55,7 @@ namespace osu.Game.Tests.Visual.Navigation
});
});
- AddAssert("dummy beatmap didn't appear", () => currentSetInfos.IsDeepEqual(beatmapSets));
+ AddAssert("dummy beatmap didn't appear", () => currentSetInfos.IsDeepEqual(beatmapSets) && currentSetInfos is not null);
}
}
}
From 428b5fad3c45a545820d3fec8e06e9ce05803020 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Sat, 29 Apr 2023 10:34:50 +0900
Subject: [PATCH 4/7] Rename test scene to explicitly mention navigation
testing
---
...ceneBeatmapEditor.cs => TestSceneBeatmapEditorNavigation.cs} | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
rename osu.Game.Tests/Visual/Navigation/{TestSceneBeatmapEditor.cs => TestSceneBeatmapEditorNavigation.cs} (96%)
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
similarity index 96%
rename from osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
rename to osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
index 9760fc2c97..e1fba1d630 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditor.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
@@ -11,7 +11,7 @@ using osu.Game.Screens.Menu;
namespace osu.Game.Tests.Visual.Navigation
{
- public partial class TestSceneBeatmapEditor : OsuGameTestScene
+ public partial class TestSceneBeatmapEditorNavigation : OsuGameTestScene
{
[Test]
public void TestCancelNavigationToEditor()
From a6f01861124b3ba119c830c43178af95945864ad Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Sat, 29 Apr 2023 10:49:25 +0900
Subject: [PATCH 5/7] Improve legibility and code quality of new test
---
.../TestSceneBeatmapEditorNavigation.cs | 48 ++++++-------------
1 file changed, 14 insertions(+), 34 deletions(-)
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
index e1fba1d630..c76758e6c6 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
@@ -16,46 +16,26 @@ namespace osu.Game.Tests.Visual.Navigation
[Test]
public void TestCancelNavigationToEditor()
{
- BeatmapSetInfo[]? beatmapSets = null;
+ BeatmapSetInfo[] beatmapSets = null!;
- AddStep("Timestamp current beatmapsets", () =>
+ AddStep("Fetch initial beatmaps", () =>
{
- Game.Realm.Run(realm =>
- {
- beatmapSets = realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray();
- });
+ beatmapSets = Game.Realm.Run(realm => realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray());
});
- AddStep("Set current beatmap to default", () =>
+ AddStep("Set current beatmap to default", () => Game.Beatmap.SetDefault());
+
+ AddStep("Push editor loader", () => Game.ScreenStack.Push(new EditorLoader()));
+ AddUntilStep("Wait for loader current", () => Game.ScreenStack.CurrentScreen is EditorLoader);
+ 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", () =>
{
- Game.Beatmap.SetDefault();
+ var beatmapSetsAfter = Game.Realm.Run(realm => realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray());
+ return beatmapSetsAfter.SequenceEqual(beatmapSets);
});
-
- AddStep("Open editor loader", () =>
- {
- Game.ScreenStack.Push(new EditorLoader());
- });
-
- AddUntilStep("wait for editor", () => Game.ScreenStack.CurrentScreen is EditorLoader);
-
- AddStep("close editor while loading", () =>
- {
- Game.ScreenStack.CurrentScreen.Exit();
- });
-
- AddUntilStep("wait for editor", () => Game.ScreenStack.CurrentScreen is MainMenu);
-
- BeatmapSetInfo[]? currentSetInfos = null;
-
- AddStep("Get current beatmaps", () =>
- {
- Game.Realm.Run(realm =>
- {
- currentSetInfos = realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray();
- });
- });
-
- AddAssert("dummy beatmap didn't appear", () => currentSetInfos.IsDeepEqual(beatmapSets) && currentSetInfos is not null);
}
}
}
From 32f8c674f4d751937db57082cc872bc38a1c7bed Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Sat, 29 Apr 2023 11:01:29 +0900
Subject: [PATCH 6/7] Extract beatmap retrieval method for more legibility
---
.../Navigation/TestSceneBeatmapEditorNavigation.cs | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
index c76758e6c6..554da36cc4 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
@@ -2,7 +2,6 @@
// See the LICENCE file in the repository root for full licence text.
using System.Linq;
-using DeepEqual.Syntax;
using NUnit.Framework;
using osu.Framework.Screens;
using osu.Game.Beatmaps;
@@ -18,10 +17,7 @@ namespace osu.Game.Tests.Visual.Navigation
{
BeatmapSetInfo[] beatmapSets = null!;
- AddStep("Fetch initial beatmaps", () =>
- {
- beatmapSets = Game.Realm.Run(realm => realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray());
- });
+ AddStep("Fetch initial beatmaps", () => beatmapSets = allBeatmapSets());
AddStep("Set current beatmap to default", () => Game.Beatmap.SetDefault());
@@ -30,12 +26,9 @@ 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", () =>
- {
- var beatmapSetsAfter = Game.Realm.Run(realm => realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray());
- return beatmapSetsAfter.SequenceEqual(beatmapSets);
- });
+ BeatmapSetInfo[] allBeatmapSets() => Game.Realm.Run(realm => realm.All<BeatmapSetInfo>().Where(x => !x.DeletePending).ToArray());
}
}
}
From 26431006448c9d192ac5931fc1fc3cf10f76c2a9 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Sat, 29 Apr 2023 11:05:10 +0900
Subject: [PATCH 7/7] Add xmldoc to new test mentioning failure rate and
general purpose
---
.../Visual/Navigation/TestSceneBeatmapEditorNavigation.cs | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
index 554da36cc4..603573058e 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs
@@ -12,6 +12,14 @@ namespace osu.Game.Tests.Visual.Navigation
{
public partial class TestSceneBeatmapEditorNavigation : OsuGameTestScene
{
+ /// <summary>
+ /// When entering the editor, a new beatmap is created as part of the asynchronous load process.
+ /// This test ensures that in the case of an early exit from the editor (ie. while it's still loading)
+ /// doesn't leave a dangling beatmap behind.
+ ///
+ /// This may not fail 100% due to timing, but has a pretty high chance of hitting a failure so works well enough
+ /// as a test.
+ /// </summary>
[Test]
public void TestCancelNavigationToEditor()
{