Skip to content
New issue

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

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

Already on GitHub? # to your account

Ensure Loader tests create deps file #112500

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Feb 12, 2025

Fixes #112471

@Copilot Copilot bot review requested due to automatic review settings February 12, 2025 20:56

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (2)
  • src/libraries/System.Runtime.Loader/tests/DefaultContext/System.Runtime.Loader.DefaultContext.Tests.csproj: Language not supported
  • src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj: Language not supported
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 12, 2025
@ericstj ericstj added area-AssemblyLoader-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 12, 2025
@@ -2,8 +2,6 @@
<PropertyGroup>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<TestRuntime>true</TestRuntime>
<!-- LoadInDefaultContext test relies on no deps.json -->
Copy link
Member

Choose a reason for hiding this comment

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

For reference, this was introduced in dotnet/corefx#39279 cc @ViktorHofer

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that also modified Reflection tests which was fixed by @steveharter in ac8ed29

I bet there was a reason for it at the time, but most likely we've changed the test runner since then.

I did a search for any other tests breaking this and didn't find any: https://github.com/search?q=repo%3Adotnet%2Fruntime+GenerateDependencyFile+path%3A*tests*csproj&type=code

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM assuming the tests pass

@ericstj
Copy link
Member Author

ericstj commented Feb 13, 2025

Tests passed for me locally. We'll see what happens in validation. Looks like things are pretty 🔥 🚒 🟥 right now.

@jkotas
Copy link
Member

jkotas commented Feb 13, 2025

/ba-g known win2025 infra issue

@jkotas jkotas merged commit 505e317 into dotnet:main Feb 13, 2025
84 of 90 checks passed
@jkotas
Copy link
Member

jkotas commented Feb 13, 2025

Thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Runtime.Loader.[DefaultContext.]Tests fail when ran from top-level script
2 participants