Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
src/tests tree test xunit-based source generated runner #60846
src/tests tree test xunit-based source generated runner #60846
Changes from 8 commits
1bbfc9c
6523a11
6143a48
9b9ae95
54e93eb
14ca067
149ffe2
23b7975
623cc48
a20864e
4a7ebe9
f5009c8
59960aa
191ac42
a2bdb78
e597fec
e783f9e
c978193
270144b
4c73d6f
e81deda
4f9dcbb
d5961f6
4d412d0
377b8a0
835f56d
c84c5d1
b7fb243
d49c1f4
1b19bed
040877c
d736995
a77d13c
04650fd
8ef73bd
5881fa1
eccf29a
ccd08a2
db68ce4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reflect on
Microsoft.DotNet.XUnitExtensions
in generator to avoid duplicating these types?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, but using the Roslyn symbol APIs to reflect like that is not fast. I think eventually making this generator live in dotnet/arcade might be a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In future versions of the generator (v1 milestone of the new test infra), we need to support spitting out an XUnit-style results file as well as basic test filtering, so living in arcade would make more sense then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be careful not to regress managed build time too significantly. While ultimately we'll optimize it by compiling tests as larger apps, it remains to be seen whether compiling tests in merged form makes sense for all pipelines - some of the GC stress pipelines come to mind where some tests are timing out already today even without merging - so that the "standalone" mode is likely to stay as a valid way of running tests at least for niche scenarios. If we pay the reflection startup cost just once per Roslyn generation of all the wrappers, it's probably just fine but paying the cost for every single test may be significant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like caching it like that due to the fact that it's technically not correct and can break some assumptions that Roslyn has about the lifetime of a generator (as Roslyn can throw them away or reuse them in many different ways including in compiler server scenarios)