Fix code review issues and add unit tests

osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckSpinnerRecoveryTimeTest.cs

@@ -0,0 +1,95 @@
+// 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.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Edit;
+using osu.Game.Rulesets.Edit.Checks.Components;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Osu.Edit.Checks;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Tests.Beatmaps;
+
+namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks
+{
+    [TestFixture]
+    public class CheckSpinnerRecoveryTimeTest
+    {
+        private CheckSpinnerRecoveryTime check = null!;
+
+        [SetUp]
+        public void Setup()
+        {
+            check = new CheckSpinnerRecoveryTime();
+        }
+
+        [Test]
+        public void TestEasyOk()
+        {
+            Assert.That(runScenario(DifficultyRating.Easy, 1000, 3000), Is.Empty);
+        }
+
+        [Test]
+        public void TestEasyFail()
+        {
+            var issues = runScenario(DifficultyRating.Easy, 1000, 2500).ToList();
+            Assert.That(issues, Has.Count.EqualTo(1));
+            Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort);
+        }
+
+        [Test]
+        public void TestNormalOk()
+        {
+            Assert.That(runScenario(DifficultyRating.Normal, 1000, 2000), Is.Empty);
+        }
+
+        [Test]
+        public void TestNormalFail()
+        {
+            var issues = runScenario(DifficultyRating.Normal, 1000, 1500).ToList();
+            Assert.That(issues, Has.Count.EqualTo(1));
+            Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort);
+        }
+
+        [Test]
+        public void TestHardOk()
+        {
+            Assert.That(runScenario(DifficultyRating.Hard, 1000, 1500), Is.Empty);
+        }
+
+        [Test]
+        public void TestHardFail()
+        {
+            var issues = runScenario(DifficultyRating.Hard, 1000, 1250).ToList();
+            Assert.That(issues, Has.Count.EqualTo(1));
+            Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort);
+        }
+
+        [Test]
+        public void TestEverythingElseOk()
+        {
+            Assert.That(runScenario(DifficultyRating.Insane, 1000, 1001), Is.Empty);
+            Assert.That(runScenario(DifficultyRating.Expert, 1000, 1001), Is.Empty);
+            Assert.That(runScenario(DifficultyRating.ExpertPlus, 1000, 1001), Is.Empty);
+        }
+
+        private IEnumerable<Issue> runScenario(DifficultyRating rating, double spinnerEnd, double nextObjectStart)
+        {
+            Spinner spinner = new Spinner { StartTime = 0, EndTime = spinnerEnd };
+            HitObject nextObject = new HitObject { StartTime = nextObjectStart };
+            var hitObjects = new List<HitObject> { spinner, nextObject };
+
+            TimingControlPoint tp = new TimingControlPoint { BeatLength = 500 };
+            ControlPointInfo controlPoints = new ControlPointInfo();
+            controlPoints.Add(0, tp);
+
+            var beatmap = new Beatmap { HitObjects = hitObjects, ControlPointInfo = controlPoints };
+            var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), rating);
+
+            return check.Run(context);
+        }
+    }
+}

osu.Game.Rulesets.Osu/Edit/Checks/CheckSpinnerRecoveryTime.cs

@@ -21,13 +21,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks
 
         public IEnumerable<Issue> Run(BeatmapVerifierContext context)
         {
-            double? recommendedRecoveryTime_ = this.recommendedRecoveryTime(context.InterpretedDifficulty);
+            double? recommendedRecoveryTime = getRecommendedRecoveryTime(context.InterpretedDifficulty);
 
-            // If anything goes, we can skip this check
-            if (recommendedRecoveryTime_ is not double recommendedRecoveryTime)
-            {
+            if (!recommendedRecoveryTime.HasValue)
                 yield break;
-            }
 
             foreach (var (firstObject, secondObject) in context.Beatmap.HitObjects.Zip(context.Beatmap.HitObjects.Skip(1)))
             {
@@ -43,7 +40,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks
                 if (timeDifferenceInBeats < recommendedRecoveryTime)
                 {
                     yield return new IssueTemplateSpinnerRecoveryTooShort(this)
-                        .Create(spinner, timeDifferenceInBeats, recommendedRecoveryTime, context.InterpretedDifficulty);
+                        .Create(spinner, timeDifferenceInBeats, recommendedRecoveryTime.Value, context.InterpretedDifficulty);
                 }
             }
 
@@ -55,7 +52,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks
         /// </summary>
         /// <param name="interpretedDifficulty">The difficulty to interpret the beatmap as</param>
         /// <returns>Number of beats if there is a guideline for it, null if anything goes</returns>
-        private double? recommendedRecoveryTime(DifficultyRating interpretedDifficulty)
+        private double? getRecommendedRecoveryTime(DifficultyRating interpretedDifficulty)
         {
             switch (interpretedDifficulty)
             {