Adding builds files to the test projects by jonmill · Pull Request #6572 · dotnet/corefx

jonmill · 2016-03-01T23:37:27Z

Adding builds files to the test projects so we will only build a test project on the platform (and only with the configuration) that it should build for.

cc @weshaggard

weshaggard · 2016-03-02T07:19:16Z

@sokket As we talked about last time you shouldn't include TargetGroup configurations in the builds file right now as the tests currently don't support anything beyond the default empty TargetGroup. So please exclude any configurations that define a TargetGroup from the builds files.

jonmill · 2016-03-02T18:18:52Z

Sorry about that, I changed my script and it included the TargetGroup nodes...I'll fix it now

ellismg · 2016-03-03T19:31:08Z

Something goofy is going on. The Winodws_NT legs failed because they were running at least some tests that were marked as AnyUnix.

jonmill · 2016-03-04T00:03:02Z

@ellismg it looks like there isn't any xunit filtering done per-OS...the ZipFile tests had the same problem:

CoreRun.exe xunit.console.netcore.exe System.IO.Compression.ZipFile.Tests.dll -xml testResults.xml -notrait Benchmark=true -notrait category=OuterLoop -notrait category=failing

@weshaggard what generates the xunit command to run? It looks like we need to add the -notrait category=<os> trait

ellismg · 2016-03-04T00:04:57Z

@weshaggard what generates the xunit command to run? It looks like we need to add the -notrait category= trait

It's handled by tests.targets in build tools.

ellismg · 2016-03-04T00:06:36Z

run-test.sh will also need to be updated since it looks like the paths of stuff in the output folder are changing, and previously it assumed that there would be just one OS folder that all tests went into.

jonmill · 2016-03-04T00:06:49Z

@ellismg Thanks for pointing me to that. Maybe that file should be changed to filter based on the OS that it is running on instead of the OSGroup parameter?

ellismg · 2016-03-04T00:17:34Z

Maybe that file should be changed to filter based on the OS that it is running on instead of the OSGroup parameter?

The only issue with that is I don't believe today we have a property that describes the OS your are running on. AFAIK we only distinguish between "Windows" and "Unix" for the host OS, so we'd need to add something and then plumb things through.

jonmill · 2016-03-04T18:47:41Z

Makes sense... @weshaggard any thoughts on if this will be the right approach?

jonmill · 2016-03-07T18:44:10Z

@weshaggard The problem with any of the variables I found is that they do not match up with our test case filters seen here...for instance, we turn FreeBSD distros into Ubuntu runtime but have BSD test filters. I think we will need to plumb something fully through the system to get the right traits

jonmill · 2016-03-07T19:42:30Z

Adding some logging to check the state of MSBuild variables on all the platforms...

jonmill · 2016-03-07T22:35:45Z

Windows failure is #6732, will retest since it shouldn't be a 100% repro

jonmill · 2016-03-09T19:39:47Z

@chcosta FYI

chcosta · 2016-03-09T20:05:37Z

dir.targets

+    <TestFilterCategory Condition="'$(OS)' == 'Linux'">-notrait category=nonlinuxtests</TestFilterCategory>
+    <TestFilterCategory Condition="'$(OS)' == 'FreeBSD'">-notrait category=nonfreebsdtests</TestFilterCategory>
+    <TestFilterCategory Condition="'$(OS)' == 'NetBSD'">-notrait category=nonnetbsdtests</TestFilterCategory>
+    <TestCommandLine>$(TestCommandLine) $(TestFilterCategory)</TestCommandLine>


I'm grabbing a copy of this code change to dir.targets, as-is, for a change I am currently working on. I'll be sure to cc you on the PR so that you are aware of any conflicts if you make additional changes. This change is simpler than the build tools change I was considering, only because coordination between buildtools and corefx won't be required. Is there a plan to move this into BuildTools? This seems somewhat redundant and likely to cause conflicts at some point given that these are additive and keying off of different properties...

https://github.com/dotnet/buildtools/blob/master/src/Microsoft.DotNet.Build.Tasks/PackageFiles/tests.targets#L60

When I last spoke with @weshaggard he mentioned that he usually puts changes like this into CoreFx to vet it first and then moves it over to build tools after.

Yes I'm cool with staging this in corefx and then prompting to buildtools.

As for the specific change I'm not entirely sure $(OS) is the right way to go here. Msbuild will currently only set it to Unix, OSX, or Windows (https://github.com/radical/msbuild/blob/fix-isosx/src/Shared/NativeMethodsShared.cs#L534). I think we probably should start passing it through the builds file as TestOSGroup or something like that which we can default to the OSGroup passed in at the root or we can start explicitly passing it.

jonmill · 2016-03-17T22:21:08Z

Some infrastructure failures, sent emails about them. Will retest

jonmill · 2016-03-22T23:01:22Z

Machines went offline during the build...retrying

jonmill · 2016-03-22T23:01:28Z

@dotnet-bot test this please

mmitche · 2016-03-22T23:05:32Z

Stand by, I'm restarting the system with an update or two

jonmill · 2016-03-22T23:41:58Z

@mmitche cool, let me know when you've completed the update so I can re-test?

mmitche · 2016-03-23T00:08:14Z

@dotnet-bot test this please

jonmill · 2016-03-25T17:12:20Z

@weshaggard Things are green! Can you take another look please?

weshaggard · 2016-03-25T21:15:49Z

dir.targets

    <TargetFrameworkProfile></TargetFrameworkProfile>
    <!-- We set this property to avoid MSBuild errors regarding not setting TargetFrameworkProfile (see above line) -->
    <PortableNuGetMode>true</PortableNuGetMode>
+


This needs to be reconciled with @Priya91 recent BuildTools PR dotnet/buildtools#564

Looks like I can just use TargetOS instead

When will the BuildTools change be picked up by CoreFx?

I think the TargetOS part is already included, but @Priya91 is also working merging another change with #7255.

weshaggard · 2016-03-25T22:45:33Z

src/System.Diagnostics.Debug/src/System.Diagnostics.Debug.csproj

 <Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
-    <Configuration Condition="'$(Configuration)'==''">Windows_Debug</Configuration>
+    <Configuration Condition="'$(Configuration)'==''">$(TargetOS)_Debug</Configuration>


TargetOS isn't set by the time this is evaluated so you are essentially setting it to _Debug which isn't going to help.

Changed to $(OSEnvironment)

jonmill · 2016-03-30T22:05:06Z

Looks like network timeouts. Trying tests again

jonmill · 2016-03-31T04:23:08Z

@dotnet-bot test Innerloop CentOS7.1 Debug Build and Test

jonmill · 2016-03-31T17:01:28Z

@weshaggard Any further feedback or can I merge this change?

weshaggard · 2016-03-31T17:17:34Z

build.proj

-    <TestOsTarget Condition="'$(OSGroup)'==''">$(OSEnvironment)</TestOsTarget>
+  </PropertyGroup>
+  <PropertyGroup>
+    <InputOSGroup>$(OSGroup)</InputOSGroup>


InputOSGroup - Are you trying to use this to capture what is passed at the command line?

Yes. It will capture whatever is put from the command line OR it will generate it based on the current running platform. This gives me a variable that is guaranteed to be there and have data so I can use it anywhere in the csproj

jonmill · 2016-03-31T17:23:11Z

FYI @maririos

weshaggard · 2016-03-31T17:39:59Z

src/System.Net.Sockets/src/System.Net.Sockets.csproj

 <Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
-    <Configuration Condition="'$(Configuration)'==''">Windows_Debug</Configuration>
+    <Configuration Condition=" '$(Configuration)' == '' ">$(OS)_Debug</Configuration>


Lets try this for now but I suspect this isn't going to give us the full experience we want in all scenario.

I can probably change that to $(InputOSGroup)...that would give the desired effect

Ah no it won't, nevermind

Yes it won't but I also don't have a better answer here right now either.

weshaggard · 2016-03-31T17:45:14Z

OK after the removal of the OS fallback please squash and then LGTM

Thanks for tackling this.

jonmill · 2016-03-31T18:41:38Z

Merging. Thanks everyone!

Since dotnet#6572 was merged, we only build test projects for the platforms they test. This means the UnsupportedPlatforms metadata is no longer needed (instead using the .builds file to control when the project is built is the right thing to do). Leverage this in run-test.sh, we now simply loop over all the tests in the relevent folders under bin/tests, depending on the platform in question.

Adding builds files to the test projects Commit migrated from dotnet/corefx@e8326c3

Since dotnet/corefx#6572 was merged, we only build test projects for the platforms they test. This means the UnsupportedPlatforms metadata is no longer needed (instead using the .builds file to control when the project is built is the right thing to do). Leverage this in run-test.sh, we now simply loop over all the tests in the relevent folders under bin/tests, depending on the platform in question. Commit migrated from dotnet/corefx@001983b

dnfclas added the cla-already-signed label Mar 1, 2016

jonmill added the 2 - In Progress label Mar 1, 2016

chcosta reviewed Mar 9, 2016
View reviewed changes

mmitche closed this Mar 22, 2016

mmitche reopened this Mar 22, 2016

dnfclas added the cla-already-signed label Mar 22, 2016

mmitche added 2 - In Progress and removed 2 - In Progress labels Mar 22, 2016

weshaggard reviewed Mar 25, 2016
View reviewed changes

weshaggard reviewed Mar 31, 2016
View reviewed changes

Adding builds files for the test projects

7525994

jonmill merged commit e8326c3 into dotnet:master Mar 31, 2016

jonmill deleted the builds branch March 31, 2016 18:41

jonmill removed the 2 - In Progress label Mar 31, 2016

ellismg mentioned this pull request Apr 5, 2016

Remove "UnsupportedPlatforms" metadata #7508

Closed

ellismg mentioned this pull request Apr 5, 2016

Remove "UnsupportedPlatforms" metadata #7523

Merged

This was referenced Apr 13, 2016

April13 script sync dotnet/wcf#1009

Merged

Find test builds files deeper in file structure. #7747

Merged

karelz modified the milestone: 1.0.0-rtm Dec 3, 2016

joperezr mentioned this pull request Jan 31, 2020

Non-Windows builds are broken due to recent change in Microsoft.VisualBasic.builds file dotnet/runtime#16871

Closed

macrogreg pushed a commit to open-telemetry/opentelemetry-dotnet-instrumentation that referenced this pull request Sep 24, 2020

Merge pull request dotnet/corefx#6572 from sokket/builds

01a9a58

Adding builds files to the test projects Commit migrated from dotnet/corefx@e8326c3

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Merge pull request dotnet/corefx#6572 from sokket/builds

0e43cc6

Adding builds files to the test projects Commit migrated from dotnet/corefx@e8326c3

Conversation

jonmill commented Mar 1, 2016

Uh oh!

weshaggard commented Mar 2, 2016

Uh oh!

jonmill commented Mar 2, 2016

Uh oh!

ellismg commented Mar 3, 2016

Uh oh!

jonmill commented Mar 4, 2016

Uh oh!

ellismg commented Mar 4, 2016

Uh oh!

ellismg commented Mar 4, 2016

Uh oh!

jonmill commented Mar 4, 2016

Uh oh!

ellismg commented Mar 4, 2016

Uh oh!

jonmill commented Mar 4, 2016

Uh oh!

jonmill commented Mar 7, 2016

Uh oh!

jonmill commented Mar 7, 2016

Uh oh!

jonmill commented Mar 7, 2016

Uh oh!

jonmill commented Mar 9, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonmill commented Mar 17, 2016

Uh oh!

jonmill commented Mar 22, 2016

Uh oh!

jonmill commented Mar 22, 2016

Uh oh!

mmitche commented Mar 22, 2016

Uh oh!

jonmill commented Mar 22, 2016

Uh oh!

mmitche commented Mar 23, 2016

Uh oh!

jonmill commented Mar 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonmill commented Mar 30, 2016

Uh oh!

jonmill commented Mar 31, 2016

Uh oh!

jonmill commented Mar 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonmill commented Mar 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment