Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DllImportGenerator unit tests' compilations use live refs #63410

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jan 5, 2022

  • Add property (TestRunRequiresLiveRefPack) to indicate live refs should be copied to test output, such that they get included in the helix work item payload.
  • Make DllImportGenerator unit tests use the live-built references during test run when compiling for the latest .NET version

Resolves #60605

@AaronRobinsonMSFT @jkoritzinsky

@ghost ghost assigned elinor-fung Jan 5, 2022
@elinor-fung elinor-fung added this to the 7.0.0 milestone Jan 5, 2022
@elinor-fung elinor-fung marked this pull request as ready for review January 5, 2022 23:12
@AaronRobinsonMSFT
Copy link
Member

@ericstj @danmoseley @stephentoub @safern - this approach is what all source generators should use for unit testing. The DllImport source generator has unit tests that check several snippets and this approach allows us to add new APIs in a PR and immediately test them using the live build.

@elinor-fung elinor-fung merged commit 44b9a6e into dotnet:main Jan 6, 2022
@elinor-fung elinor-fung deleted the testCompileWithLiveRef branch January 6, 2022 19:37
@ericstj
Copy link
Member

ericstj commented Jan 6, 2022

this approach is what all source generators should use for unit testing

Today other source generators are running the generators on the build machine rather than the test machine.

<ProjectReference Include="..\..\gen\System.Text.Json.SourceGeneration.Roslyn4.0.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />

Is there a reason you decided to run the compiler on the test machine instead? Were you wanted to generate test code on the fly?

cc @eerhardt @layomia

@stephentoub
Copy link
Member

Today other source generators are running the generators on the build machine rather than the test machine

The regex tests are executing the generator on the test machine, building against the implementation assemblies to avoid the hassle of using refs.

@jkoritzinsky
Copy link
Member

We wanted to follow Roslyn best practice and use the Roslyn SDK testing APIs (at least the ones that were available when we started working on the source generator) to test our generator and analyzers.

We have separate tests that run the generator on the build machine, but we wanted some tests that ran as unit tests because the debugging experience is significantly better in VS when your entrypoint is an xunit test.

@AaronRobinsonMSFT
Copy link
Member

We wanted to follow Roslyn best practice and use the Roslyn SDK testing APIs (at least the ones that were available when we started working on the source generator) to test our generator and analyzers.

This was the key point for us. The Roslyn team's "best practices" were our guide in most if not all decisions and this fell out of that.

/cc @jaredpar @chsienki

@ericstj
Copy link
Member

ericstj commented Jan 20, 2022

@elinor-fung -- those are the basic unit tests that are less sensitive to references. I believe their meant to excercise the basic functionality of the generator and cover edge cases like invalid syntax etc that doesn't build correctly.

There's a larger test suite that covers the behavior of runtime with generated code that runs the generator on the build machine. Those actually compile and execute the result. I believe we found it simpler to compile those on the build machine since it makes the tests simpler: they look more like the customer code.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable using live ref pack in source generator unit tests
5 participants