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

Clean up usings in test cases assembly #88678

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

mrvoorhe
Copy link
Contributor

The main goal here is to get rid of the unused System.Linq usings. These cause problems for our UnityLinker tests that run against mono class libraries as it requires an additional reference. We could work around it by referencing everything in the mono class libraries, which is what illink does during it's tests. That said, some of the files had a lot of unnecessary usings and I don't think it's a bad thing to clean them up anyways.

Rather than hunt down the 10-20 files that were problematic I ran Rider's namespace clean up functionality on Mono.Linker.Tests.Cases

@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Jul 11, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 11, 2023
@ghost
Copy link

ghost commented Jul 11, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

The main goal here is to get rid of the unused System.Linq usings. These cause problems for our UnityLinker tests that run against mono class libraries as it requires an additional reference. We could work around it by referencing everything in the mono class libraries, which is what illink does during it's tests. That said, some of the files had a lot of unnecessary usings and I don't think it's a bad thing to clean them up anyways.

Rather than hunt down the 10-20 files that were problematic I ran Rider's namespace clean up functionality on Mono.Linker.Tests.Cases

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework

Milestone: -

@vitek-karas
Copy link
Member

There's something weird about the test assembly. VS refuses to cleanup any usings (it behaves as if all of them are necessary even though if I remove one which is unused everything works). And apparently Rider goes to the other extreme and removes even those which are needed (as seen by the failing tests). Oh well :-(

@mrvoorhe
Copy link
Contributor Author

Tests with #if threw Rider's logic off. Reverted changes in failing tests. And I did a manual review of other tests with #if to see if there were any incorrect changes. I reverted some of these. Worst case, the risk is I break a test when ran with the mono class libraries. Which will be my problem to fix down the road when we pull in these changes to UnityLinker

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the cleanup!

@mrvoorhe
Copy link
Contributor Author

@vitek-karas I don't understand what the Build Analysis failure is about. Is there something wrong?

@vitek-karas
Copy link
Member

It's a check which looks through all of the failures and matches them against known bugs. It needs to wait for all the runs to finish - unfortunately the wasi leg seems to be stuck. I'll give it a bit more time and if that doesn't help we can merge regardless as nothing else seems to be failing.

The main goal here is to get rid of the unused `System.Linq` usings.  These cause problems for our UnityLinker tests that run against mono class libraries as it requires an additional reference.  We could work around it by referencing everything in the mono class libraries, which is what illink does during it's tests.  That said, some of the files had a lot of unnecessary usings and I don't think it's a bad thing to clean them up anyways.

Rather than hunt down the 10-20 files that were problematic I ran Rider's namespace clean up functionality on `Mono.Linker.Tests.Cases`
@vitek-karas
Copy link
Member

Test failures are unrelated. Most of the failures have since been fixed by #88751.

@vitek-karas vitek-karas merged commit 06b696c into dotnet:main Jul 13, 2023
50 of 55 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection.Metadata community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants