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

Enable 4 more libraries tests #73214

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

MichalStrehovsky
Copy link
Member

No description provided.

@ghost
Copy link

ghost commented Aug 2, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: MichalStrehovsky
Assignees: MichalStrehovsky
Labels:

area-Infrastructure-libraries

Milestone: -

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

<Application>
<Assembly Name="System.Linq.Tests">
<Type Name="System.Linq.Tests.MinTests">
<Method Name="MinBy_Generic_RunOnce_HasExpectedOutput" Dynamic="Required All">
Copy link
Member

Choose a reason for hiding this comment

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

Having to duplicate all of the test names into this default.rd.xml file seems very fragile. Anyone refactoring / renaming / etc. these tests is very likely to miss the fact that this file will also need to be updated. There's no better way to achieve the same goal?

Copy link
Member

@jkotas jkotas Aug 2, 2022

Choose a reason for hiding this comment

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

Maybe we can have a dummy dynamically unreachable call in WrapArgs to ensure sure that the right method instantiations are available at runtime?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ultimate fix is to stop using generic methods as Fact/Theory since it's unsupportable with source generated runners where we would eventually like to go to (except maybe some subset that can be statically analyzed - xunit does a lot of runtime conversions for these).

If this is blocking, I'm honestly just going to create an issue to bring up System.Linq and keep the test disabled. This file took forever to make and I just want it to be over for my own sanity. I still have 80 test assemblies to go over for .NET 7 and this test is low value - the only value it has is extra random code to compile and run, but there are no NativeAOT-specific code paths

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky MichalStrehovsky merged commit 32b473d into dotnet:main Aug 4, 2022
@MichalStrehovsky MichalStrehovsky deleted the moretests4 branch August 4, 2022 06:41
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants