Enable tests to compile with netstandard #15956

tarekgh · 2017-02-08T19:57:42Z

Most of the tests have netstandard configurations and it doesn't compile. the changes here is to fix the configurations and the tests to compile.
This is important step to get netfx tests compile too as we are in most of the time netfx fallback to netstandard configuration

The changes include some fixes to compile teh sources too.

Most of the tests have netstandard configurations and it doesn't compile. the changes here is to fix the configurations and the tests to compile. This is important step to get netfx tests compile too as we are in most of the time netfx fallback to netstandard configuration The changes include some fixes to compile teh sources too.

tarekgh · 2017-02-08T19:59:28Z

@weshaggard could you please have a look?

Note this step to get the tests compile fine with netstandard but I believe we need to have a look later to the projects which not having netstandard configurations and decide if we need to enable it to other configurations (e.g netfx).

FYI to @danmosemsft

tarekgh · 2017-02-08T20:01:33Z

src/System.Collections.Immutable/tests/Configurations.props

@@ -2,7 +2,6 @@
 <Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <BuildConfigurations>
-      netstandard;


@weshaggard ignore removing this one. I'll try to get this back as I am ifdef the code specific to netcoreapp

tarekgh · 2017-02-08T20:26:16Z

@JeremyKuhne it looks the System.IO.Prots tests not stable.

https://ci.dot.net/job/dotnet_corefx/job/master/job/windows_nt_debug_prtest/3121/

https://ci.dot.net/job/dotnet_corefx/job/master/job/windows_nt_debug_prtest/3121/testReport/junit/(root)/ReadTimeout_Property/ReadTimeout_0_ReadByte_BeforeOpen/

tarekgh · 2017-02-08T20:34:41Z

test Innerloop Windows_NT Debug Build and Test please

JeremyKuhne · 2017-02-08T21:10:04Z

it looks the System.IO.Prots tests not stable.

Thanks for the heads up. @willdean We should open specific issues for the tests and work through them. I'll disable this particular one.

12:08:09         ERROR!!! Read Method ReadByte timed out in 637ms expected something less then 100ms
12:08:09         Expected: True
12:08:09         Actual:   False
12:08:09         Stack Trace:
12:08:09            D:\j\workspace\windows_nt_de---4526f5ff\src\System.IO.Ports\tests\Legacy\Support\PortsTest.cs(24,0): at System.IO.PortsTests.PortsTest.Fail(String format, Object[] args)
12:08:09            D:\j\workspace\windows_nt_de---4526f5ff\src\System.IO.Ports\tests\Legacy\SerialPort\ReadTimeout.cs(301,0): at ReadTimeout_Property.VerifyZeroTimeout(SerialPort com, ReadMethodDelegate readMethod)
12:08:09            D:\j\workspace\windows_nt_de---4526f5ff\src\System.IO.Ports\tests\Legacy\SerialPort\ReadTimeout.cs(243,0): at ReadTimeout_Property.VerifyZeroTimeoutBeforeOpen(ReadMethodDelegate readMethod)
12:08:09            D:\j\workspace\windows_nt_de---4526f5ff\src\System.IO.Ports\tests\Legacy\SerialPort\ReadTimeout.cs(127,0): at ReadTimeout_Property.ReadTimeout_0_ReadByte_BeforeOpen()

tarekgh · 2017-02-08T21:15:07Z

@JeremyKuhne the failed are 3 tests.

ReadTimeout_Property.ReadTimeout_0_ReadByte_BeforeOpen
ReadTimeout_Property.ReadTimeout_0_Read_byte_int_int_AfterOpen
ReadTimeout_Property.ReadTimeout_0_Read_byte_int_int_BeforeOpen

JeremyKuhne · 2017-02-08T21:17:01Z

I'm going to put all of the ReadTimeout tests in an issue. I assume we need to have very generous thresholds and as such they'll need to move (at least in part) to outer loop.

weshaggard · 2017-02-08T21:20:52Z

Tools-Override/ConstructSharedFx.targets

@@ -18,7 +18,7 @@
  -->  
  <Target Name="ConstructTestingHost"
          BeforeTargets="Build;BuildAllProjects"
-          Condition="!Exists('$(ConstructSharedFxSem)') OR '$(ReconstructSharedFx)' == 'true'" >
+          Condition="(!Exists('$(ConstructSharedFxSem)') OR '$(ReconstructSharedFx)' == 'true') And '$(TargetGroup)' != 'netfx'" >


I recently deleted this file so you can undo this change.

I am seeing the conflict :-)

weshaggard · 2017-02-08T21:22:48Z

src/System.Diagnostics.DiagnosticSource/ref/Configurations.props

      netstandard;
-      netstandard1.1;


Did you figure out why the netstandard1.1 configuration was picked? I'm mainly interested because we might need to add it back at some point.

I didn't look at this yet but I'll try later as I am curious too.

weshaggard · 2017-02-08T21:23:30Z

src/System.Diagnostics.DiagnosticSource/src/Configurations.props

@@ -2,7 +2,6 @@
 <Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <BuildConfigurations>
-      net45-Windows_NT;
      netstandard1.1;


If you are removing the netstandard1.1 ref you should probably do the same for the src.

willdean · 2017-02-08T21:26:57Z

@JeremyKuhne There are loads of tests playing games with StopWatch which might have this problem if the VM decides to run very slowly - I hadn't seen any sign of this particular group causing problems on previous CI runs, but to have an issue to dump slow tests into seems like a good idea.

In reality we could relax the max time limits in these test without necessarily making the tests run any slower on average, they would just have more room if the VM did go slow.

…ests

weshaggard · 2017-02-08T21:35:34Z

src/System.Runtime/tests/System/EnumTests.cs

@@ -62,6 +62,7 @@ public static IEnumerable<object[]> Parse_TestData()
            yield return new object[] { "1", false, UInt64Enum.One };
            yield return new object[] { "5", false, (UInt64Enum)5 };

+#if netcoreapp


Are these #ifdef'ed because of missing APIs or because of differences in behavior? If it is API this is fine but we should consider splitting them into another file instead of a ton of #ifdef's. If it is behavior differences then we need to file bugs and get them investigated further as we shouldn't blindly disable large sets of tests for that.

it is because missing APIs. do you want me split the test in this PR or we can do it later?

I split the code to its own files

weshaggard · 2017-02-08T21:36:07Z

src/System.Runtime/tests/System.Runtime.Tests.csproj

@@ -6,15 +6,17 @@
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <NoWarn>1718</NoWarn>
    <DefineConstants Condition="'$(TargetGroup)'=='netstandard'">$(DefineConstants);netstandard17</DefineConstants>


Do we still need netstandard17 in this project?

tarekgh · 2017-02-08T23:32:46Z

@weshaggard I am merging this one. if you have any more comments I can address it in another PR. thanks for reviewing it.

* Enable tests to compile with netstandard Most of the tests have netstandard configurations and it doesn't compile. the changes here is to fix the configurations and the tests to compile. This is important step to get netfx tests compile too as we are in most of the time netfx fallback to netstandard configuration The changes include some fixes to compile teh sources too. * Fix System.Collections.Immutable * update src configuration * remove netstandard17 defines * Split netcoreapp code to its own files * Remove 17 from the test class name Commit migrated from dotnet/corefx@15b4f51

dnfclas added the cla-already-signed label Feb 8, 2017

tarekgh commented Feb 8, 2017

View reviewed changes

tarekgh added the area-Meta label Feb 8, 2017

tarekgh self-assigned this Feb 8, 2017

Fix System.Collections.Immutable

668d6fa

weshaggard reviewed Feb 8, 2017

View reviewed changes

Merge branch 'master' of https://github.com/dotnet/corefx into NetFXT…

3e98372

…ests

weshaggard reviewed Feb 8, 2017

View reviewed changes

update src configuration

21a2b80

weshaggard reviewed Feb 8, 2017

View reviewed changes

tarekgh added 3 commits February 8, 2017 13:48

remove netstandard17 defines

701815a

Split netcoreapp code to its own files

75c8d08

Remove 17 from the test class name

c7908da

tarekgh merged commit 15b4f51 into dotnet:master Feb 8, 2017

karelz modified the milestone: 2.0.0 Feb 9, 2017

tarekgh deleted the NetFXTests branch October 27, 2017 17:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tests to compile with netstandard #15956

Enable tests to compile with netstandard #15956

tarekgh commented Feb 8, 2017

tarekgh commented Feb 8, 2017

tarekgh Feb 8, 2017 •

edited

tarekgh commented Feb 8, 2017 •

edited

tarekgh commented Feb 8, 2017

JeremyKuhne commented Feb 8, 2017

tarekgh commented Feb 8, 2017

JeremyKuhne commented Feb 8, 2017

weshaggard Feb 8, 2017

tarekgh Feb 8, 2017

weshaggard Feb 8, 2017

tarekgh Feb 8, 2017

weshaggard Feb 8, 2017

willdean commented Feb 8, 2017

weshaggard Feb 8, 2017

tarekgh Feb 8, 2017

tarekgh Feb 8, 2017

weshaggard Feb 8, 2017

tarekgh commented Feb 8, 2017

Enable tests to compile with netstandard #15956

Enable tests to compile with netstandard #15956

Conversation

tarekgh commented Feb 8, 2017

tarekgh commented Feb 8, 2017

tarekgh Feb 8, 2017 • edited

Choose a reason for hiding this comment

tarekgh commented Feb 8, 2017 • edited

tarekgh commented Feb 8, 2017

JeremyKuhne commented Feb 8, 2017

tarekgh commented Feb 8, 2017

JeremyKuhne commented Feb 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willdean commented Feb 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh commented Feb 8, 2017

tarekgh Feb 8, 2017 •

edited

tarekgh commented Feb 8, 2017 •

edited