From 7162ce6e72dbbfd0f1af3e0088d1a69dae423612 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Tue, 22 Sep 2015 10:32:24 +0200 Subject: [PATCH] Clarify why convention tests are failing --- ...dDebuggerDisplayAttributeValueException.cs | 18 +++++++ .../InvalidDebuggerDisplayReturnType.cs | 18 +++++++ ...issingDebuggerDisplayAttributeException.cs | 10 ++++ ...MissingDebuggerDisplayPropertyException.cs | 10 ++++ .../MutableModelPropertiesException.cs | 21 ++++++++ Octokit.Tests.Conventions/ModelTests.cs | 50 ++++++++++++++++--- .../Octokit.Tests.Conventions.csproj | 5 ++ Octokit.Tests.Conventions/TypeExtensions.cs | 14 ++++++ Octokit.Tests/Helpers/AssertEx.cs | 19 ------- 9 files changed, 139 insertions(+), 26 deletions(-) create mode 100644 Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayAttributeValueException.cs create mode 100644 Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayReturnType.cs create mode 100644 Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayAttributeException.cs create mode 100644 Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayPropertyException.cs create mode 100644 Octokit.Tests.Conventions/Exception/MutableModelPropertiesException.cs diff --git a/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayAttributeValueException.cs b/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayAttributeValueException.cs new file mode 100644 index 0000000000..9f25afdea4 --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayAttributeValueException.cs @@ -0,0 +1,18 @@ +using System; + +namespace Octokit.Tests.Conventions +{ + public class InvalidDebuggerDisplayAttributeValueException : Exception + { + public InvalidDebuggerDisplayAttributeValueException(Type modelType, string value) + : base (CreateMessage(modelType, value)) { } + + static string CreateMessage(Type modelType, string value) + { + return string.Format( + "Model type '{0}' has invalid DebuggerDisplayAttribute value '{1}'. Expected '{{DebuggerDisplay, nq}}'", + modelType.FullName, + value); + } + } +} \ No newline at end of file diff --git a/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayReturnType.cs b/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayReturnType.cs new file mode 100644 index 0000000000..3f9fab9046 --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/InvalidDebuggerDisplayReturnType.cs @@ -0,0 +1,18 @@ +using System; + +namespace Octokit.Tests.Conventions +{ + public class InvalidDebuggerDisplayReturnType : Exception + { + public InvalidDebuggerDisplayReturnType(Type modelType, Type propertyType) + : base (CreateMessage(modelType, propertyType)) { } + + static string CreateMessage(Type modelType, Type propertyType) + { + return string.Format( + "Model type '{0}' has invalid DebuggerDisplay return type '{1}'. Expected 'string'.", + modelType.FullName, + propertyType.Name); + } + } +} \ No newline at end of file diff --git a/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayAttributeException.cs b/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayAttributeException.cs new file mode 100644 index 0000000000..47e94e295b --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayAttributeException.cs @@ -0,0 +1,10 @@ +using System; + +namespace Octokit.Tests.Conventions +{ + public class MissingDebuggerDisplayAttributeException : Exception + { + public MissingDebuggerDisplayAttributeException(Type modelType) + : base (string.Format("Model type '{0}' is missing the DebuggerDisplayAttribute.", modelType.FullName)) { } + } +} \ No newline at end of file diff --git a/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayPropertyException.cs b/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayPropertyException.cs new file mode 100644 index 0000000000..0587364e5b --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/MissingDebuggerDisplayPropertyException.cs @@ -0,0 +1,10 @@ +using System; + +namespace Octokit.Tests.Conventions +{ + public class MissingDebuggerDisplayPropertyException : Exception + { + public MissingDebuggerDisplayPropertyException(Type modelType) + : base (string.Format("Model type '{0}' is missing the DebuggerDisplay property.", modelType.FullName)) { } + } +} \ No newline at end of file diff --git a/Octokit.Tests.Conventions/Exception/MutableModelPropertiesException.cs b/Octokit.Tests.Conventions/Exception/MutableModelPropertiesException.cs new file mode 100644 index 0000000000..66d23a42b0 --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/MutableModelPropertiesException.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace Octokit.Tests.Conventions +{ + public class MutableModelPropertiesException : Exception + { + public MutableModelPropertiesException(Type modelType, IEnumerable mutableProperties) + : base (CreateMessage(modelType, mutableProperties)) { } + + static string CreateMessage(Type modelType, IEnumerable mutableProperties) + { + return string.Format("Model type '{0}' contains the following mutable properties: {1}{2}", + modelType.FullName, + Environment.NewLine, + string.Join(Environment.NewLine, mutableProperties.Select(x => x.Name))); + } + } +} \ No newline at end of file diff --git a/Octokit.Tests.Conventions/ModelTests.cs b/Octokit.Tests.Conventions/ModelTests.cs index 926485fa7e..f126e8cd57 100644 --- a/Octokit.Tests.Conventions/ModelTests.cs +++ b/Octokit.Tests.Conventions/ModelTests.cs @@ -2,7 +2,6 @@ using System.Collections; using System.Diagnostics; using System.Linq; -using Octokit.Tests.Helpers; using Xunit; using System.Collections.Generic; using System.Reflection; @@ -15,25 +14,50 @@ public class ModelTests [MemberData("ModelTypes")] public void AllModelsHaveDebuggerDisplayAttribute(Type modelType) { - var attribute = AssertEx.HasAttribute(modelType); + var attribute = modelType.GetCustomAttribute(inherit: false); + if (attribute == null) + { + throw new MissingDebuggerDisplayAttributeException(modelType); + } - Assert.Equal("{DebuggerDisplay,nq}", attribute.Value); + if (attribute.Value != "{DebuggerDisplay,nq}") + { + throw new InvalidDebuggerDisplayAttributeValueException(modelType, attribute.Value); + } var property = modelType.GetProperty("DebuggerDisplay", BindingFlags.Instance | BindingFlags.NonPublic); + if (property == null) + { + throw new MissingDebuggerDisplayPropertyException(modelType); + } - Assert.NotNull(property); - Assert.Equal(typeof(string), property.PropertyType); + if (property.PropertyType != typeof(string)) + { + throw new InvalidDebuggerDisplayReturnType(modelType, property.PropertyType); + } } [Theory] [MemberData("ResponseModelTypes")] public void ResponseModelsHaveGetterOnlyProperties(Type modelType) { + var mutableProperties = new List(); + foreach (var property in modelType.GetProperties()) { var setter = property.GetSetMethod(nonPublic: true); - Assert.True(setter == null || !setter.IsPublic); + if (setter == null || !setter.IsPublic) + { + continue; + } + + mutableProperties.Add(property); + } + + if (mutableProperties.Any()) + { + throw new MutableModelPropertiesException(modelType, mutableProperties); } } @@ -41,6 +65,8 @@ public void ResponseModelsHaveGetterOnlyProperties(Type modelType) [MemberData("ResponseModelTypes")] public void ResponseModelsHaveReadOnlyCollections(Type modelType) { + var mutableCollectionProperties = new List(); + foreach (var property in modelType.GetProperties()) { var propertyType = property.PropertyType; @@ -54,9 +80,19 @@ public void ResponseModelsHaveReadOnlyCollections(Type modelType) continue; } - AssertEx.IsReadOnlyCollection(propertyType); + if (propertyType.IsReadOnlyCollection()) + { + continue; + } + + mutableCollectionProperties.Add(property); } } + + if (mutableCollectionProperties.Any()) + { + throw new MutableModelPropertiesException(modelType, mutableCollectionProperties); + } } //TODO: This should (probably) be moved to the PaginationTests class that is being introduced in PR #760 diff --git a/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj b/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj index dfcd229b9b..0d78e345a7 100644 --- a/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj +++ b/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj @@ -58,6 +58,11 @@ + + + + + diff --git a/Octokit.Tests.Conventions/TypeExtensions.cs b/Octokit.Tests.Conventions/TypeExtensions.cs index f5342f9fea..be3e589b41 100644 --- a/Octokit.Tests.Conventions/TypeExtensions.cs +++ b/Octokit.Tests.Conventions/TypeExtensions.cs @@ -99,6 +99,20 @@ public static Type GetGenericArgument(this Type type) { return type.GetGenericArguments()[0]; } + + public static bool IsReadOnlyCollection(this Type type) + { + var isReadOnlyList = type.HasGenericTypeDefinition(typeof(IReadOnlyList<>)); + + var isReadOnlyDictionary = type.HasGenericTypeDefinition(typeof(IReadOnlyDictionary<,>)); + + return isReadOnlyList || isReadOnlyDictionary; + } + + private static bool HasGenericTypeDefinition(this Type type, Type genericTypeDefinition) + { + return type.IsGenericType && type.GetGenericTypeDefinition() == genericTypeDefinition; + } } public enum TypeCategory { Other, Task, GenericTask, ReadOnlyList, ClientInterface } diff --git a/Octokit.Tests/Helpers/AssertEx.cs b/Octokit.Tests/Helpers/AssertEx.cs index b9c1041b14..bce0debf40 100644 --- a/Octokit.Tests/Helpers/AssertEx.cs +++ b/Octokit.Tests/Helpers/AssertEx.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Reflection; using System.Threading.Tasks; using Xunit; @@ -14,15 +13,6 @@ public static void WithMessage(Action assert, string message) assert(); } - public static TAttribute HasAttribute(MemberInfo memberInfo, bool inherit = false) where TAttribute : Attribute - { - var attribute = memberInfo.GetCustomAttribute(inherit); - - Assert.NotNull(attribute); - - return attribute; - } - static readonly string[] whitespaceArguments = { " ", "\t", "\n", "\n\r", " " }; public static async Task ThrowsWhenGivenWhitespaceArgument(Func action) @@ -39,14 +29,5 @@ public static void IsReadOnlyCollection(object instance) // The collection == null case is for .NET 4.0 Assert.True(instance is IReadOnlyList && (collection == null || collection.IsReadOnly)); } - - public static void IsReadOnlyCollection(Type type) - { - var isReadOnlyList = type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IReadOnlyList<>); - - var isReadOnlyDictionary = type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IReadOnlyDictionary<,>); - - Assert.True(isReadOnlyList || isReadOnlyDictionary); - } } }