Skip to content

Conversation

@KevinRansom
Copy link
Contributor

  1. During the build build a nuget package with the FSharp.Core unit tests that the coreclr team can use to verify they haven't broken us again.
  2. FSharp.Core on master branch puts the net45 resources in the wrong place.
  3. Add support for xunit to FSharp.Core unit tests, because coreclr uses xunit.
  4. Ensure that tests can be built with an out of the box VS, and not require any VFS repo build magic.
  5. Remove the app.config for the unittests, they are no longer needed.

@KevinRansom KevinRansom requested a review from brettfo January 28, 2019 07:53
@KevinRansom KevinRansom force-pushed the testsforcoreclr branch 5 times, most recently from 60ef93e to afe7cd1 Compare February 7, 2019 01:48

<ItemGroup>
<PackageReference Include="XliffTasks" Version="$(XliffTasksPackageVersion)" />
<PackageReference Include="XliffTasks" Version="$(XliffTasksPackageVersion)" PrivateAssets="All" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

</None>

<Content Include="$(MSBuildProjectDirectory)\**\*.*"
Exclude="$(MSBuildProjectDirectory)\bin\**\*.*;$(MSBuildProjectDirectory)\obj\**\*.*;$(MSBuildProjectDirectory)\FSharp.Core\SampleTuples\**\*.*;$(MSBuildProjectDirectory)\Directory.Build.props;$(MSBuildProjectDirectory)\Directory.Build.targes;">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: Directory.BUild targes -> Directory.Build.targets

More importantly, what specifically are you trying to include? This line looks terrible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to include the SampleTuples subdirectory. And the intermediate and output directories, which probably now are correctly in the artifacts directory.

</PropertyGroup>

<PropertyGroup>
<DefineConstants Condition="'$(UnitTestType)' == 'xunit'">$(DefineConstants);XUNIT</DefineConstants>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're explicitly setting $(UnitTestType) to xunit a few lines up, so this section doesn't really do anything.

<Pattern1>FSCoreVersion</Pattern1>
<Replacement1>$(FSCoreVersion)</Replacement1>
</CopyAndSubstituteText>
<Compile Include="SurfaceArea.coreclr.fs" Condition="$(TargetFramework.StartsWith('netcoreapp')) == true and '$(FX_VERIFY_SURFACEAREA)' == 'true'" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

== true makes me sad.


<ItemGroup>
<ProjectReference Include="$(FSharpSourcesRoot)\fsharp\FSharp.Core\FSharp.Core.fsproj" />
<Content Include="**/*" Exclude="**/*.bak;Directory.Build.Props;Directory.Build.targets" CopyToOutputDirectory="never" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What specifically would you like to include?

@KevinRansom KevinRansom reopened this Feb 20, 2019
@KevinRansom KevinRansom reopened this Feb 21, 2019
@KevinRansom KevinRansom reopened this Feb 21, 2019
@KevinRansom KevinRansom merged commit bead034 into dotnet:master Feb 21, 2019
@KevinRansom KevinRansom deleted the testsforcoreclr branch July 2, 2019 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants