Skip to content

Conversation

@trylek
Copy link
Member

@trylek trylek commented Nov 4, 2021

This change is based on Jeremy's POC PR; I have modified the build
scripts to minimize the changes to the individual test build scripts.
In the simplest case the conversion requires just replacing the Main
method with a [Fact]-marked test entrypoint and removing the
OutputType property from the project file. I have demonstrated the
transformation on the project DecimalTest.

Empty OutputType property gets automatically replaced with the Library
default in the SDK scripts; this is not a huge deal but I had to
remove the pre-existing hack of OutputType=Library + CLRTestKind=""
implying SharedLibrary. According to my search most Library projects
already have the SharedLibrary CLRTestKind set, for the rest I plan
to set it using a bulk project modification; I think I should be able
to adapt the ILTransform analyzer for this purpose.

Thanks

Tomas

Contributes to: #54512

jkoritzinsky and others added 14 commits November 4, 2021 20:32
…nt Main method that runs all static parameterless [Fact] attributes in sequence, wrapped in a simple try-catch.
… basic cases of OS and runtime-specific filtering.
…ames for tests like the TypeGenerator suite). Add support for theories to handle the RunOnly tests that aren't failure cases and don't require out-of-proc execution.
Co-authored-by: Alexander Köplinger <[email protected]>
This change is based on Jeremy's POC PR; I have modified the build
scripts to minimize the changes to the individual test build scripts.
In the simplest case the conversion requires just replacing the Main
method with a [Fact]-marked test entrypoint and removing the
OutputType property from the project file.

Empty OutputType property gets automatically replaced with the Library
default in the SDK scripts; this is not a huge deal but I had to
remove the pre-existing hack of OutputType=Library + CLRTestKind=""
implying SharedLibrary. According to my search most Library projects
already have the SharedLibrary CLRTestKind set, for the rest I plan
to set it using a bulk project modification; I think I should be able
to adapt the ILTransform analyzer for this purpose.

Thanks

Tomas
@trylek trylek requested a review from jkoritzinsky November 4, 2021 21:53
@ghost
Copy link

ghost commented Nov 4, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Comment on lines +82 to +88
<!-- Inject the XUnit wrapper generator into test build for [Fact]-style tests -->
<ItemGroup Condition="$(IsFactStyleTest)">
<ProjectReference Include="$(MSBuildThisFileDirectory)/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
</ItemGroup>

<Import Project="Common/XUnitWrapperGenerator/XUnitWrapperGenerator.props" Condition="$(IsFactStyleTest)" />

Copy link
Member

Choose a reason for hiding this comment

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

We can reference the generator and import the MSBuild file for all tests. The generator should be a no-op for non-Fact-style tests. If it isn't, I can make changes to the generator to fix that.

@trylek
Copy link
Member Author

trylek commented Nov 12, 2021

Closing as Jeremy is currently code reviewing the real change. My contribution here is very minor, involving just the subtle tweaks to src/tests/Directory.Build.targets and I have asked Jeremy to integrate that delta into his PR for the sake of consistency.

@trylek trylek closed this Nov 12, 2021
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Nov 16, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2021
@trylek trylek deleted the xunit-runner-generator branch May 10, 2023 18:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants