Fix broken JSON deserialization in .NET 4.5 (#1647)

* Cross target Octokit.Tests against netcoreapp1.0 and net452

* Add net45-specific references

This fixes a build error after adding the net452 target:
error CS0656: Missing compiler required member 'Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create'

* Add SimpleJson conditional compilation symbols for net45

* Disable AppDomain when running tests

Thanks to Dominick and Patrik: https://twitter.com/leastprivilege/status/893376624233762816

* Use nameof operator instead of magic strings

* Remove conditional compilation symbols as they are not used in the conventions tests project

* Enable cross targetting in the conventions tests project

* Run tests against netcoreapp1.0 only when not on Windows

* Going too fast bites you

Octokit.Tests.Conventions/ClientConstructorTests.cs

@@ -9,7 +9,7 @@ namespace Octokit.Tests.Conventions
     public class ClientConstructorTests
     {
         [Theory]
-        [MemberData("GetTestConstructorClasses")]
+        [MemberData(nameof(GetTestConstructorClasses))]
         public void CheckTestConstructorNames(Type type)
         {
             const string constructorTestClassName = "TheCtor";

Octokit.Tests.Conventions/ModelTests.cs

@@ -14,7 +14,7 @@ namespace Octokit.Tests.Conventions
         private static readonly Assembly Octokit = typeof(AuthorizationUpdate).GetTypeInfo().Assembly;
 
         [Theory]
-        [MemberData("ModelTypes")]
+        [MemberData(nameof(ModelTypes))]
         public void AllModelsHaveDebuggerDisplayAttribute(Type modelType)
         {
             var attribute = modelType.GetTypeInfo().GetCustomAttribute<DebuggerDisplayAttribute>(inherit: false);
@@ -41,7 +41,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("ResponseModelTypes")]
+        [MemberData(nameof(ResponseModelTypes))]
         public void AllResponseModelsHavePublicParameterlessCtors(Type modelType)
         {
             var ctor = modelType.GetConstructor(Type.EmptyTypes);
@@ -53,7 +53,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("ResponseModelTypes")]
+        [MemberData(nameof(ResponseModelTypes))]
         public void ResponseModelsHaveGetterOnlyProperties(Type modelType)
         {
             var mutableProperties = new List<PropertyInfo>();
@@ -77,7 +77,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("ResponseModelTypes")]
+        [MemberData(nameof(ResponseModelTypes))]
         public void ResponseModelsHaveReadOnlyCollections(Type modelType)
         {
             var mutableCollectionProperties = new List<PropertyInfo>();
@@ -111,7 +111,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("ResponseModelTypes")]
+        [MemberData(nameof(ResponseModelTypes))]
         public void ResponseModelsUseStringEnumWrapper(Type modelType)
         {
             var enumProperties = modelType.GetProperties()
@@ -124,7 +124,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("ModelTypesWithUrlProperties")]
+        [MemberData(nameof(ModelTypesWithUrlProperties))]
         public void ModelsHaveUrlPropertiesOfTypeString(Type modelType)
         {
             var propertiesWithInvalidType = modelType
@@ -140,7 +140,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("EnumTypes")]
+        [MemberData(nameof(EnumTypes))]
         public void EnumMembersHaveParameterAttribute(Type enumType)
         {
             if (enumType == typeof(Language))

Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj

@@ -4,7 +4,7 @@
     <Description>Convention-based tests for Octokit</Description>
     <AssemblyTitle>Octokit.Tests.Conventions</AssemblyTitle>
     <Authors>GitHub</Authors>
-    <TargetFramework>netcoreapp1.0</TargetFramework>
+    <TargetFrameworks>netcoreapp1.0;net452</TargetFrameworks>
     <NoWarn>$(NoWarn);CS4014;CS1998</NoWarn>
     <AssemblyName>Octokit.Tests.Conventions</AssemblyName>
     <PackageId>Octokit.Tests.Conventions</PackageId>
@@ -24,18 +24,17 @@
   <ItemGroup>
     <Compile Include="..\Octokit.Tests\Helpers\AssertEx.cs" />
     <None Include="app.config" />
+    <Content Include="xunit.runner.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">
+  <ItemGroup>
     <ProjectReference Include="..\Octokit\Octokit.csproj" />
     <ProjectReference Include="..\Octokit.Reactive\Octokit.Reactive.csproj" />
     <ProjectReference Include="..\Octokit.Tests\Octokit.Tests.csproj" />
   </ItemGroup>
 
-  <PropertyGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">
-    <DefineConstants>$(DefineConstants);NO_SERIALIZABLE;HAS_TYPEINFO</DefineConstants>
-  </PropertyGroup>
-
   <ItemGroup>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
     <PackageReference Include="xunit" Version="2.2.0" />

Octokit.Tests.Conventions/PaginationTests.cs

@@ -9,7 +9,7 @@ namespace Octokit.Tests.Conventions
     public class PaginationTests
     {
         [Theory(Skip = "Enable this to run it and find all the places where things break")]
-        [MemberData("GetClientInterfaces")]
+        [MemberData(nameof(GetClientInterfaces))]
         public void CheckObservableClients(Type clientInterface)
         {
             var methodsOrdered = clientInterface.GetMethodsOrdered();
@@ -28,7 +28,7 @@ namespace Octokit.Tests.Conventions
         }
 
         [Theory]
-        [MemberData("GetClientInterfaces")]
+        [MemberData(nameof(GetClientInterfaces))]
         public void CheckPaginationGetAllMethodNames(Type clientInterface)
         {
             var methodsOrdered = clientInterface.GetMethodsOrdered();

Octokit.Tests.Conventions/SyncObservableClients.cs

@@ -11,7 +11,7 @@ namespace Octokit.Tests.Conventions
     public class SyncObservableClients
     {
         [Theory]
-        [MemberData("GetClientInterfaces")]
+        [MemberData(nameof(GetClientInterfaces))]
         public void CheckObservableClients(Type clientInterface)
         {
             var observableClient = clientInterface.GetObservableClientInterface();

Octokit.Tests.Conventions/xunit.runner.json

@@ -0,0 +1,3 @@
+{
+    "appDomain": "denied"
+}

Octokit.Tests/Octokit.Tests.csproj

@@ -4,7 +4,7 @@
     <Description>Tests for Octokit</Description>
     <AssemblyTitle>Octokit.Tests</AssemblyTitle>
     <Authors>GitHub</Authors>
-    <TargetFramework>netcoreapp1.0</TargetFramework>
+    <TargetFrameworks>netcoreapp1.0;net452</TargetFrameworks>
     <NoWarn>$(NoWarn);CS4014;CS1998</NoWarn>
     <AssemblyName>Octokit.Tests</AssemblyName>
     <PackageId>Octokit.Tests</PackageId>
@@ -23,9 +23,12 @@
 
   <ItemGroup>
     <None Include="app.config" />
+    <Content Include="xunit.runner.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">
+  <ItemGroup>
     <ProjectReference Include="..\Octokit\Octokit.csproj" />
     <ProjectReference Include="..\Octokit.Reactive\Octokit.Reactive.csproj" />
     <PackageReference Include="NSubstitute" Version="2.0.2" />
@@ -41,6 +44,11 @@
     <PackageReference Include="xunit.runner.visualstudio" Version="2.2.0" />
   </ItemGroup>
 
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net452' ">
+    <Reference Include="System" />
+    <Reference Include="Microsoft.CSharp" />
+  </ItemGroup>
+
   <ItemGroup>
     <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
   </ItemGroup>

Octokit.Tests/xunit.runner.json

@@ -0,0 +1,3 @@
+{
+    "appDomain": "denied"
+}

Octokit/Octokit.csproj

@@ -24,7 +24,7 @@
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(TargetFramework)' == 'net45' ">
-    <DefineConstants>$(DefineConstants);HAS_ENVIRONMENT;HAS_REGEX_COMPILED_OPTIONS</DefineConstants>
+    <DefineConstants>$(DefineConstants);HAS_ENVIRONMENT;HAS_REGEX_COMPILED_OPTIONS;SIMPLE_JSON_INTERNAL,SIMPLE_JSON_OBJARRAYINTERNAL,SIMPLE_JSON_READONLY_COLLECTIONS</DefineConstants>
   </PropertyGroup>
 
   <ItemGroup Condition=" '$(TargetFramework)' == 'net45' ">

build/Context.cs

@@ -1,3 +1,6 @@
+using Cake.Common;
+using Cake.Common.Diagnostics;
+using Cake.Common.Tools.DotNetCore.Test;
 using Cake.Core;
 using Cake.Core.IO;
 using Cake.Frosting;
@@ -23,6 +26,26 @@ public class Context : FrostingContext
 
     public Project[] Projects { get; set; }
 
+    public DotNetCoreTestSettings GetTestSettings()
+    {
+        var settings = new DotNetCoreTestSettings
+        {
+            Configuration = Configuration,
+            NoBuild = true,
+            Verbose = false
+        };
+
+        if (!this.IsRunningOnWindows())
+        {
+            var testFramework = "netcoreapp1.0";
+
+            this.Information($"Running tests against {testFramework} only as we're not on Windows.");
+            settings.Framework = testFramework;
+        }
+
+        return settings;
+    }
+
     public Context(ICakeContext context)
         : base(context)
     {

build/Tasks/ConventionTests.cs

@@ -1,7 +1,6 @@
 using System.Linq;
 using Cake.Common.Diagnostics;
 using Cake.Common.Tools.DotNetCore;
-using Cake.Common.Tools.DotNetCore.Test;
 using Cake.Frosting;
 
 [Dependency(typeof(Build))]
@@ -12,12 +11,7 @@ public sealed class ConventionTests : FrostingTask<Context>
         foreach (var project in context.Projects.Where(x => x.ConventionTests))
         {
             context.Information("Executing Convention Tests Project {0}...", project.Name);
-            context.DotNetCoreTest(project.Path.FullPath, new DotNetCoreTestSettings
-            {
-                Configuration = context.Configuration,
-                NoBuild = true,
-                Verbose = false
-            });
+            context.DotNetCoreTest(project.Path.FullPath, context.GetTestSettings());
         }
     }
 }

build/Tasks/UnitTests.cs

@@ -1,7 +1,6 @@
 using System.Linq;
 using Cake.Common.Diagnostics;
 using Cake.Common.Tools.DotNetCore;
-using Cake.Common.Tools.DotNetCore.Test;
 using Cake.Frosting;
 
 [Dependency(typeof(Build))]
@@ -12,12 +11,7 @@ public sealed class UnitTests : FrostingTask<Context>
         foreach (var project in context.Projects.Where(x => x.UnitTests))
         {
             context.Information("Executing Unit Tests Project {0}...", project.Name);
-            context.DotNetCoreTest(project.Path.FullPath, new DotNetCoreTestSettings
-            {
-                Configuration = context.Configuration,
-                NoBuild = true,
-                Verbose = false
-            });
+            context.DotNetCoreTest(project.Path.FullPath, context.GetTestSettings());
         }
     }
 }