Skip to content

Conversation

Youssef1313
Copy link
Member

No description provided.

@Youssef1313 Youssef1313 marked this pull request as ready for review August 14, 2025 20:25
@Youssef1313
Copy link
Member Author

@marcpopMSFT @MiYanni Can you please take a look? Thanks!

@marcpopMSFT
Copy link
Member

Looks like two classes of changes. One is that the test assemblies all need to be exes. That should be fine here but if we move to v3 in other repos, we'll have to double check no one is assuming the app type.

The second is providing additional info in the custom facts. This appears to line up with the guidance in https://xunit.net/xunit.analyzers/rules/xUnit3003

@Youssef1313
Copy link
Member Author

but if we move to v3 in other repos, we'll have to double check no one is assuming the app type.

If a a project forgets to set OutputType exe, there should be an error saying that.

@Youssef1313
Copy link
Member Author

The second is providing additional info in the custom facts. This appears to line up with the guidance in https://xunit.net/xunit.analyzers/rules/xUnit3003

Yup. The caller info provided through attribute constructor is used mostly when running in Test Explorer. This avoids the hassle of how Test Explorer should calculate the file and line where a test is declared. Historically this was by reading PDB, then Roslyn exposed a helper to Test Explorer that does most a syntactic search (thus, it can fail in inheritance scenarios). We found it most ideal to do via caller info.

@Youssef1313
Copy link
Member Author

@marcpopMSFT Is it good merge then? 🎉

@marcpopMSFT marcpopMSFT merged commit fd7fb46 into dotnet:main Aug 18, 2025
8 checks passed
@Youssef1313 Youssef1313 deleted the xunit-v3-mtp branch August 19, 2025 06:19
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