-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 debugger attributes tests. #89011
Clean up debugger attributes tests. #89011
Conversation
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar Issue Details
|
@vitek-karas Do I have to do something to get the tests to rerun? The failure about
Seems out of date. I deleted that line. |
Note the error is for a file in |
I did not see that coming! A copy of |
* Remove `SetupLinkerKeepDebugMembersAttribute`. This isn't supported anymore by the linker. * Remove NETCOREAPP behavior difference in debugger attribute removal tests * Move `DebuggerDisplayAttributeOnTypeWithNonExistentMethod` to the `KeepDebugMembers` folder since that is the scenario this test is testing
b356e61
to
e38c157
Compare
Yeah - it's good and bad. It's good since we share the tests between illink and NativeAOT. It's bad because it's the same mess as in illink, just slightly different. When I get some time I want to:
|
I think that's worth doing. If the file names had been different that might have been enough to get my brain to notice it was a different copy of
I didn't look it too closely but maybe you could share more following the pattern we do with UnityLinker's test suite. We inherit and override things.
We have a lot of the same files. They just inherit and extend rather than copy whole sale from upstream. Then again, maybe you're use case is different enough that trying to follow the same pattern would lead to more api breaks for us, in which case feel free to stick with copying whole sale 😄 |
@vitek-karas I think these changes are good now? |
In the product we went with source sharing and partial types typically. But it was mostly for performance reasons to avoid casting a virtual calls everywhere. I would need to look into this some more which approach would work best for the tests. |
Remove
SetupLinkerKeepDebugMembersAttribute
. This isn't supported anymore by the linker.Remove NETCOREAPP behavior difference in debugger attribute removal tests
Move
DebuggerDisplayAttributeOnTypeWithNonExistentMethod
to theKeepDebugMembers
folder since that is the scenario this test is testing