From 60378de7785b7b914d218ccfacd216a44a0c2a73 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 19 May 2024 11:25:06 +0800 Subject: [PATCH 1/6] Move get test from project into individual method. --- .../Extensions.cs | 27 +++++++++++++++++++ .../TestTestClass.cs | 16 +---------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs b/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs index 9aa0fbd5c..2b118dc3c 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs @@ -8,12 +8,39 @@ using ArchUnitNET.Domain; using ArchUnitNET.Domain.Extensions; using NUnit.Framework; +using osu.Game.Rulesets.Karaoke.Tests; using Attribute = ArchUnitNET.Domain.Attribute; namespace osu.Game.Rulesets.Karaoke.Architectures; public static class Extensions { + #region Architecture + + public static IEnumerable GetAllTestClass(this Architecture architecture) + { + var testProject = new Project.KaraokeTestAttribute(); + var testClasses = architecture.Classes.Where(x => + { + if (x.Namespace.RelativeNameStartsWith(testProject, "Helper")) + return false; + + return x.Namespace.RelativeNameStartsWith(testProject, ""); + }) + .Except(new[] + { + architecture.GetClassOfType(typeof(KaraokeTestBrowser)), + architecture.GetClassOfType(typeof(VisualTestRunner)), + }).ToArray(); + + if (testClasses.Length == 0) + throw new InvalidOperationException("No test class found in the project."); + + return testClasses; + } + + #endregion + #region Class public static IEnumerable GetInnerClasses(this Class @class) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs b/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs index 6627a043e..6a3f0c70f 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs @@ -5,7 +5,6 @@ using ArchUnitNET.Domain.Extensions; using NUnit.Framework; using osu.Framework.Testing; -using osu.Game.Rulesets.Karaoke.Tests; using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Karaoke.Architectures; @@ -32,23 +31,10 @@ public void CheckTestClassAndMethodNaming() var headlessTestScene = architecture.GetAttributeOfType(typeof(HeadlessTestAttribute)); // get all test classes - var testClasses = architecture.Classes.Where(x => - { - if (x.Namespace.RelativeNameStartsWith(GetExecuteProject(), "Helper")) - return false; - - return x.Namespace.RelativeNameStartsWith(GetExecuteProject(), ""); - }) - .Except(new[] - { - architecture.GetClassOfType(typeof(KaraokeTestBrowser)), - architecture.GetClassOfType(typeof(VisualTestRunner)), - }).ToArray(); + var testClasses = architecture.GetAllTestClass().ToArray(); Assertion(() => { - Assert.NotZero(testClasses.Length, "No test class found"); - foreach (var testClass in testClasses) { var testMethods = testClass.GetAllTestMembers(architecture).ToArray(); From 103631fdff64715fd7d801379efb0b2c6ee5da03 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 19 May 2024 11:58:48 +0800 Subject: [PATCH 2/6] Add the test class to check the assert using. --- .../TestTestClass.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs b/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs index 6a3f0c70f..c05cc284a 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/TestTestClass.cs @@ -1,10 +1,13 @@ // Copyright (c) andy840119 . Licensed under the GPL Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; using System.Linq; +using ArchUnitNET.Domain; using ArchUnitNET.Domain.Extensions; using NUnit.Framework; using osu.Framework.Testing; +using osu.Game.Rulesets.Karaoke.Tests; using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Karaoke.Architectures; @@ -74,4 +77,25 @@ public void CheckTestClassAndMethodNaming() } }); } + + [Test] + [Project.KaraokeTest(true)] + public void CheckAssertion() + { + var architecture = GetProjectArchitecture(); + var tests = architecture.GetAllTestClass(); + var assert = architecture.GetClassOfType(typeof(Assert)); + + Assertion(() => + { + foreach (var test in tests) + { + var assertions = test.GetCalledMethods().Where(x => EqualityComparer.Default.Equals(x.DeclaringType, assert)).ToArray(); + + // Assertion. + Assert.IsFalse(assertions.Any(x => x.NameStartsWith("True")), $"Should use {nameof(Assert.IsTrue)} instead of {nameof(Assert.True)} in the {test}."); + Assert.IsFalse(assertions.Any(x => x.NameStartsWith("False")), $"Should use {nameof(Assert.IsFalse)} instead of {nameof(Assert.False)} in the {test}."); + } + }); + } } From 68eaff69977af579b32c39bd458472960a3ec754 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 19 May 2024 11:59:05 +0800 Subject: [PATCH 3/6] Fix the broken part. --- .../Edit/Checks/TestCheck.cs | 12 ++++++------ osu.Game.Rulesets.Karaoke.Architectures/TestClass.cs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheck.cs b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheck.cs index f86db6ed5..5616b7d1e 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheck.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheck.cs @@ -34,8 +34,8 @@ public void CheckCheckClassNamingAndInherit() foreach (var checkClass in issues) { - Assert.True(checkClass.InheritedClasses.Contains(baseIssue), $"Class inherit is invalid: {checkClass}"); - Assert.True(checkClass.NameEndsWith("Issue"), $"Class name is invalid: {checkClass}"); + Assert.IsTrue(checkClass.InheritedClasses.Contains(baseIssue), $"Class inherit is invalid: {checkClass}"); + Assert.IsTrue(checkClass.NameEndsWith("Issue"), $"Class name is invalid: {checkClass}"); } // checks @@ -43,8 +43,8 @@ public void CheckCheckClassNamingAndInherit() foreach (var check in checks) { - Assert.True(check.ImplementsInterface(baseCheck), $"Class inherit is invalid: {check}"); - Assert.True(check.NameStartsWith("Check"), $"Class name is invalid: {check}"); + Assert.IsTrue(check.ImplementsInterface(baseCheck), $"Class inherit is invalid: {check}"); + Assert.IsTrue(check.NameStartsWith("Check"), $"Class name is invalid: {check}"); } // issue templates. @@ -52,8 +52,8 @@ public void CheckCheckClassNamingAndInherit() foreach (var checkClass in issueTemplates) { - Assert.True(checkClass.InheritedClasses.Contains(baseIssueTemplate), $"Class inherit is invalid: {checkClass}"); - Assert.True(checkClass.NameStartsWith("IssueTemplate"), $"Class name is invalid: {checkClass}"); + Assert.IsTrue(checkClass.InheritedClasses.Contains(baseIssueTemplate), $"Class inherit is invalid: {checkClass}"); + Assert.IsTrue(checkClass.NameStartsWith("IssueTemplate"), $"Class name is invalid: {checkClass}"); } }); } diff --git a/osu.Game.Rulesets.Karaoke.Architectures/TestClass.cs b/osu.Game.Rulesets.Karaoke.Architectures/TestClass.cs index 0469bc7e2..3958d8017 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/TestClass.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/TestClass.cs @@ -42,7 +42,7 @@ public void CheckAbstractClassLocation() if (allChildClasses.Length == 0) continue; - Assert.True(isAllowAbstractClassPosition(abstractClass, allChildClasses), $"Those child class: \n{string.Join('\n', allChildClasses.Select(x => x.ToString()))}\n\n is not in the child namespace of: \n{abstractClass}"); + Assert.IsTrue(isAllowAbstractClassPosition(abstractClass, allChildClasses), $"Those child class: \n{string.Join('\n', allChildClasses.Select(x => x.ToString()))}\n\n is not in the child namespace of: \n{abstractClass}"); } }); return; From 446710295c0a3bbbac8ea26ff757e1a452f12a95 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 19 May 2024 13:23:16 +0800 Subject: [PATCH 4/6] Adjust the test to check the test method and naming. --- .../Edit/Checks/TestCheckTest.cs | 32 +++++++++++++++++-- .../Extensions.cs | 6 ++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs index c3f93d33e..5721fd9d7 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs @@ -5,6 +5,7 @@ using ArchUnitNET.Domain; using ArchUnitNET.Domain.Extensions; using NUnit.Framework; +using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; namespace osu.Game.Rulesets.Karaoke.Architectures.Edit.Checks; @@ -47,9 +48,10 @@ public void CheckShouldContainsTest() [Test] [Project.KaraokeTest(true)] - public void CheckTestMethod() + public void CheckTestClassAndMethod() { - var architecture = GetProjectArchitecture(); + var architecture = GetProjectArchitecture(new Project.KaraokeAttribute()); + var baseCheck = architecture.GetInterfaceOfType(typeof(ICheck)); var baseCheckTest = architecture.GetClassOfType(typeof(BaseCheckTest<>)); var assertOkMethod = baseCheckTest.GetMethodMembersContainsName("AssertOk").FirstOrDefault(); @@ -66,17 +68,41 @@ public void CheckTestMethod() foreach (var checkTest in allCheckTests) { + // check the class naming. + Assert.IsTrue(isTestClassValid(checkTest, baseCheck), $"Test class {checkTest} should have correct naming"); + + // check the test method naming in the test case. var testMethods = checkTest.GetAllTestMembers(architecture).ToArray(); - Assert.NotZero(testMethods.Length, $"No test method in the {checkTest}"); + Assert.NotZero(testMethods.Length, $"No test method in the {checkTest}"); foreach (var testMethod in testMethods) { + Assert.IsTrue(isTestNamingValid(testMethod), $"Test method {testMethod} should have correct naming"); Assert.IsTrue(isTestMethod(testMethod), $"Test method {testMethod} should call {assertOkMethod} or {assertNotOkMethod} method."); } } return; + static bool isTestClassValid(Class testClass, Interface baseCheck) + { + var testCheck = testClass.GetGenericTypes().OfType().First(x => x.ImplementsInterface(baseCheck)); + return testClass.NameStartsWith(testCheck.Name); + } + + static bool isTestNamingValid(IMember testMethod) + { + var calledMethods = testMethod.GetMethodCallDependencies().FirstOrDefault(x => x.TargetMember.NameStartsWith("AssertNotOk")); + + if (calledMethods != null) + { + // todo: should get the generic type from the AssertNotOk method. + return testMethod.NameStartsWith("IssueTemplate"); + } + + return true; + } + static bool isTestMethod(IMember testMethod) { var calledMethods = testMethod.GetCalledMethods().ToArray(); diff --git a/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs b/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs index 2b118dc3c..aa6eee30b 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/Extensions.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Text.RegularExpressions; using ArchUnitNET.Domain; +using ArchUnitNET.Domain.Dependencies; using ArchUnitNET.Domain.Extensions; using NUnit.Framework; using osu.Game.Rulesets.Karaoke.Tests; @@ -67,6 +68,11 @@ public static bool HasAttributeInSelfOrChild(this Class @class, Attribute attrib #region Name + public static IEnumerable GetGenericTypes(this Class @class) + { + return @class.GetInheritsBaseClassDependencies().SelectMany(x => x.TargetGenericArguments.Select(arg => arg.Type)); + } + public static bool RelativeNameStartsWith( this IHasName cls, Project.ProjectAttribute project, From fddc2826aaaf27fb7e94f059a088182b7c08f848 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 22 May 2024 21:40:02 +0800 Subject: [PATCH 5/6] Add strict check about the test and class naming. --- .../Edit/Checks/TestCheckTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs index 5721fd9d7..635feac6a 100644 --- a/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs +++ b/osu.Game.Rulesets.Karaoke.Architectures/Edit/Checks/TestCheckTest.cs @@ -96,8 +96,8 @@ static bool isTestNamingValid(IMember testMethod) if (calledMethods != null) { - // todo: should get the generic type from the AssertNotOk method. - return testMethod.NameStartsWith("IssueTemplate"); + // todo: should get the generic type from the AssertNotOk method. the end of the method should be the issue template. + return testMethod.NameStartsWith("TestCheck"); } return true; From f6a03cfcc872f46e534469919d7f21a16a74ebb1 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 22 May 2024 21:40:16 +0800 Subject: [PATCH 6/6] Fix the invalid test name. --- .../CheckBeatmapAvailableTranslatesTest.cs | 52 ++++++++++++------- .../Checks/CheckBeatmapStageInfoTest.cs | 6 +-- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapAvailableTranslatesTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapAvailableTranslatesTest.cs index 3f1126d51..8dd60aaa9 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapAvailableTranslatesTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapAvailableTranslatesTest.cs @@ -49,7 +49,20 @@ public void TestHaveLyricButNoLanguage() } [Test] - public void TestHaveLyricAndLanguage() + public void TestEveryLyricContainsTranslate() + { + var translateLanguages = new List { new("Ja-jp") }; + var beatmap = createTestingBeatmap(translateLanguages, new[] + { + createLyric(new CultureInfo("Ja-jp"), "translate1"), + createLyric(new CultureInfo("Ja-jp"), "translate2"), + }); + + AssertOk(getContext(beatmap)); + } + + [Test] + public void TestCheckMissingTranslate() { // no lyric with translate string. (should have issue) var translateLanguages = new List { new("Ja-jp") }; @@ -75,39 +88,30 @@ public void TestHaveLyricAndLanguage() createLyric(new CultureInfo("Ja-jp"), string.Empty), }); AssertNotOk(getContext(beatmap3)); + } + [Test] + public void TestCheckMissingPartialTranslate() + { // some lyric with translate string. (should have issue) + var translateLanguages = new List { new("Ja-jp") }; var beatmap4 = createTestingBeatmap(translateLanguages, new[] { createLyric(new CultureInfo("Ja-jp"), "translate1"), createLyric(new CultureInfo("Ja-jp")), }); AssertNotOk(getContext(beatmap4)); + } - // every lyric with translate string. (should not have issue) - var beatmap5 = createTestingBeatmap(translateLanguages, new[] - { - createLyric(new CultureInfo("Ja-jp"), "translate1"), - createLyric(new CultureInfo("Ja-jp"), "translate2"), - }); - AssertOk(getContext(beatmap5)); - + [Test] + public void TestCheckContainsNotListedLanguage() + { // lyric translate not listed. (should have issue) var beatmap6 = createTestingBeatmap(null, new[] { createLyric(new CultureInfo("en-US"), "translate1"), }); AssertNotOk(getContext(beatmap6)); - - static Lyric createLyric(CultureInfo? cultureInfo = null, string translate = null!) - { - var lyric = new Lyric(); - if (cultureInfo == null) - return lyric; - - lyric.Translates.Add(cultureInfo, translate); - return lyric; - } } private static IBeatmap createTestingBeatmap(List? translateLanguage, IEnumerable? lyrics) @@ -126,4 +130,14 @@ private static IBeatmap createTestingBeatmap(List? translateLanguag private static BeatmapVerifierContext getContext(IBeatmap beatmap) => new(beatmap, new TestWorkingBeatmap(beatmap)); + + private static Lyric createLyric(CultureInfo? cultureInfo = null, string translate = null!) + { + var lyric = new Lyric(); + if (cultureInfo == null) + return lyric; + + lyric.Translates.Add(cultureInfo, translate); + return lyric; + } } diff --git a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs index a345cbaa1..fb1d46105 100644 --- a/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs +++ b/osu.Game.Rulesets.Karaoke.Tests/Editor/Checks/CheckBeatmapStageInfoTest.cs @@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.Karaoke.Tests.Editor.Checks; -public class CheckBeatmapStageInfoTest : BeatmapPropertyCheckTest +public class CheckBeatmapStageInfoTest : BeatmapPropertyCheckTest { [Test] public void TestCheckNoElement() @@ -63,11 +63,11 @@ public void TestCheckMappingItemNotExist() AssertNotOk(getContext(beatmap)); } - public class TestCheckBeatmapStageInfo : CheckBeatmapStageInfo + public class CheckBeatmapStageInfo : CheckBeatmapStageInfo { protected override string Description => "Checks for testing the shared logic"; - public TestCheckBeatmapStageInfo() + public CheckBeatmapStageInfo() { // Note that we only test the lyric layout category. RegisterCategory(x => x.StyleCategory, 0);