From 89fa1be2c8d1fc38cf86dbd9dc3522c5687c151f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 25 Dec 2019 22:55:14 +0300 Subject: [PATCH 1/5] Fix download manager potentially not handling cancel requests --- osu.Game/Database/DownloadableArchiveModelManager.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 5f688c149d..71bf195a8d 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -92,8 +92,6 @@ namespace osu.Game.Database notification.CancelRequested += () => { request.Cancel(); - currentDownloads.Remove(request); - notification.State = ProgressNotificationState.Cancelled; return true; }; @@ -109,11 +107,11 @@ namespace osu.Game.Database { DownloadFailed?.Invoke(request); - if (error is OperationCanceledException) return; - - notification.State = ProgressNotificationState.Cancelled; - Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); currentDownloads.Remove(request); + notification.State = ProgressNotificationState.Cancelled; + + if (!(error is OperationCanceledException)) + Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); } } From 099b044f04c556aefe50db9ab9a40fe9dee06c12 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 27 Dec 2019 06:37:36 +0300 Subject: [PATCH 2/5] Add headless test ensuring correct cancelling download behaviour --- .../Online/TestSceneBeatmapDownloading.cs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs diff --git a/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs b/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs new file mode 100644 index 0000000000..1572b813c0 --- /dev/null +++ b/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs @@ -0,0 +1,44 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Testing; +using osu.Game.Beatmaps; +using osu.Game.Overlays.Notifications; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.Online +{ + [HeadlessTest] + public class TestSceneBeatmapManager : OsuTestScene + { + private BeatmapManager beatmaps; + private ProgressNotification recentNotification; + + private static readonly BeatmapSetInfo test_model = new BeatmapSetInfo { OnlineBeatmapSetID = 1 }; + + [BackgroundDependencyLoader] + private void load(BeatmapManager beatmaps) + { + this.beatmaps = beatmaps; + + beatmaps.PostNotification = n => recentNotification = n as ProgressNotification; + } + + [TestCase(true)] + [TestCase(false)] + public void TestCancelDownloadFromRequest(bool closeFromRequest) + { + AddStep("download beatmap", () => beatmaps.Download(test_model)); + + if (closeFromRequest) + AddStep("cancel download from request", () => beatmaps.GetExistingDownload(test_model).Cancel()); + else + AddStep("cancel download from notification", () => recentNotification.Close()); + + AddUntilStep("is removed from download list", () => beatmaps.GetExistingDownload(test_model) == null); + AddAssert("is notification cancelled", () => recentNotification.State == ProgressNotificationState.Cancelled); + } + } +} From e030266e9575f3d24be45cc8fc3b61d88a32b182 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 27 Dec 2019 06:40:01 +0300 Subject: [PATCH 3/5] Fix test name --- osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs b/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs index 1572b813c0..7887002e1f 100644 --- a/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs +++ b/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs @@ -28,7 +28,7 @@ namespace osu.Game.Tests.Online [TestCase(true)] [TestCase(false)] - public void TestCancelDownloadFromRequest(bool closeFromRequest) + public void TestCancelDownloadRequest(bool closeFromRequest) { AddStep("download beatmap", () => beatmaps.Download(test_model)); From f03f310bde651aee3a608fc0b1928dbee2ae93c0 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 27 Dec 2019 06:43:43 +0300 Subject: [PATCH 4/5] Match file name --- ...{TestSceneBeatmapDownloading.cs => TestSceneBeatmapManager.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename osu.Game.Tests/Online/{TestSceneBeatmapDownloading.cs => TestSceneBeatmapManager.cs} (100%) diff --git a/osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs b/osu.Game.Tests/Online/TestSceneBeatmapManager.cs similarity index 100% rename from osu.Game.Tests/Online/TestSceneBeatmapDownloading.cs rename to osu.Game.Tests/Online/TestSceneBeatmapManager.cs From 3ca4d1a28c7cb1fdbefa15f46a469712fe74e0a1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 7 Jan 2020 11:47:00 +0900 Subject: [PATCH 5/5] Split out tests --- .../Online/TestSceneBeatmapManager.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Online/TestSceneBeatmapManager.cs b/osu.Game.Tests/Online/TestSceneBeatmapManager.cs index 7887002e1f..0ae0186770 100644 --- a/osu.Game.Tests/Online/TestSceneBeatmapManager.cs +++ b/osu.Game.Tests/Online/TestSceneBeatmapManager.cs @@ -26,16 +26,23 @@ namespace osu.Game.Tests.Online beatmaps.PostNotification = n => recentNotification = n as ProgressNotification; } - [TestCase(true)] - [TestCase(false)] - public void TestCancelDownloadRequest(bool closeFromRequest) + [Test] + public void TestCancelDownloadFromRequest() { AddStep("download beatmap", () => beatmaps.Download(test_model)); - if (closeFromRequest) - AddStep("cancel download from request", () => beatmaps.GetExistingDownload(test_model).Cancel()); - else - AddStep("cancel download from notification", () => recentNotification.Close()); + AddStep("cancel download from request", () => beatmaps.GetExistingDownload(test_model).Cancel()); + + AddUntilStep("is removed from download list", () => beatmaps.GetExistingDownload(test_model) == null); + AddAssert("is notification cancelled", () => recentNotification.State == ProgressNotificationState.Cancelled); + } + + [Test] + public void TestCancelDownloadFromNotification() + { + AddStep("download beatmap", () => beatmaps.Download(test_model)); + + AddStep("cancel download from notification", () => recentNotification.Close()); AddUntilStep("is removed from download list", () => beatmaps.GetExistingDownload(test_model) == null); AddAssert("is notification cancelled", () => recentNotification.State == ProgressNotificationState.Cancelled);