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

Feat: extract reflection behind a provider #3801

Merged
merged 32 commits into from
Sep 21, 2024
Merged

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Sep 10, 2024

Moves reflection calls so we can plugin a sepearate IReflectionOperations and IFileOperations provider implementations that tie the existing engine to a source generated map of data. Allowing the current engine to be used together with native aot.

Some parts are disabled when running in native, such as:

  • Deployment

  • Reflection only load

  • DynamicDataOperations is introduced to encapsulate the logic that was hardcoded in TestFramework project in attribute.

  • IReflectionOperations2 internal interface is introduced to encapsulate the remaining reflection functionality that was not isolated by the public IReflectionOperations interface.

Public types in Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration were added so we can interface with a source generator. These types need to be public, but are not meant for public consumption.

@@ -1,9 +1,9 @@
<Project>
<PropertyGroup Label="Version settings">
<!-- MSTest version -->
<VersionPrefix>3.7.0</VersionPrefix>
<VersionPrefix>3.8.0</VersionPrefix>
Copy link
Member Author

Choose a reason for hiding this comment

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

this will go away before merge, but I need it for local development, and I expect some changes will be needed.

@nohwnd nohwnd marked this pull request as ready for review September 11, 2024 12:41
@@ -143,4 +147,12 @@
<!-- SDK bottom import -->
<Import Project="Sdk.targets" Sdk="MSBuild.Sdk.Extras" Condition=" '$(OS)' == 'Windows_NT' " />
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" Condition=" '$(OS)' != 'Windows_NT' " />
<ItemGroup>
Copy link
Member Author

Choose a reason for hiding this comment

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

dunno what this is about, this project is somehow busted, every time I add files the get removed from compilation. Removing this.

Copy link
Member

Choose a reason for hiding this comment

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

It's because of the line 109. @MarcoRossignoli could we workaround that and still use the MSTestVersion.cs?

Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to keep the version without reflection we need to insert a static source or a "resource"? but I don't know if at runtime we can get it also in aot mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

That file is not added to the build if not with manual *.cs at least it wasn't working without custom source addition.


namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;

internal class SourceGeneratedDynamicDataOperations : DynamicDataOperations
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be needed later for data driven tests probably, having extra class allows me to have all the initializations wired correctly already.

Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Can you please try to reduce new public API to the minimal? Can you also seal as much as possible?

I'll need a second review and a few manual tests

@@ -143,4 +147,12 @@
<!-- SDK bottom import -->
<Import Project="Sdk.targets" Sdk="MSBuild.Sdk.Extras" Condition=" '$(OS)' == 'Windows_NT' " />
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" Condition=" '$(OS)' != 'Windows_NT' " />
<ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

It's because of the line 109. @MarcoRossignoli could we workaround that and still use the MSTestVersion.cs?

src/Adapter/MSTest.TestAdapter/PlatformServiceProvider.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/PlatformServiceProvider.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/PlatformServiceProvider.cs Outdated Show resolved Hide resolved
Comment on lines +2 to +41
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.ReflectionMetadataHook
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.Assembly.get -> System.Reflection.Assembly!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.Assembly.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.AssemblyAttributes.get -> object![]!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.AssemblyAttributes.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.AssemblyName.get -> string!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.AssemblyName.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.MyConstructorInfo
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.MyConstructorInfo.Invoker.get -> System.Func<object?[]!, object!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.MyConstructorInfo.MyConstructorInfo() -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.MyConstructorInfo.Parameters.get -> System.Type![]!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.SourceGeneratedReflectionDataProvider() -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeAttributes.get -> System.Collections.Generic.Dictionary<System.Type!, System.Attribute![]!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeAttributes.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeConstructors.get -> System.Collections.Generic.Dictionary<System.Type!, System.Reflection.ConstructorInfo![]!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeConstructors.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeConstructorsInvoker.get -> System.Collections.Generic.Dictionary<System.Type!, Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.MyConstructorInfo![]!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeConstructorsInvoker.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation.FileName.get -> string!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation.FileName.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation.MethodLocations.get -> System.Collections.Generic.Dictionary<string!, int>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation.MethodLocations.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation.TypeLocation() -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethodAttributes.get -> System.Collections.Generic.Dictionary<System.Type!, System.Collections.Generic.Dictionary<string!, System.Attribute![]!>!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethodAttributes.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethodLocations.get -> System.Collections.Generic.Dictionary<string!, Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeLocation!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethodLocations.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethods.get -> System.Collections.Generic.Dictionary<System.Type!, System.Reflection.MethodInfo![]!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeMethods.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeProperties.get -> System.Collections.Generic.Dictionary<System.Type!, System.Reflection.PropertyInfo![]!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypeProperties.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypePropertiesByName.get -> System.Collections.Generic.Dictionary<System.Type!, System.Collections.Generic.Dictionary<string!, System.Reflection.PropertyInfo!>!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypePropertiesByName.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.Types.get -> System.Type![]!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.Types.init -> void
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypesByName.get -> System.Collections.Generic.Dictionary<string!, System.Type!>!
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider.TypesByName.init -> void
static Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.ReflectionMetadataHook.SetMetadata(Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.SourceGeneration.SourceGeneratedReflectionDataProvider! metadata) -> void
Copy link
Member

Choose a reason for hiding this comment

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

Could we avoid to have this public?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we can, the types need to be accessible to the source generated assembly, and we don't know what that will be so we cannot give it IVTs.

@Evangelink
Copy link
Member

@nohwnd Can you please revert the verison changes so we merge this PR?!

eng/Versions.props Outdated Show resolved Hide resolved
eng/Versions.props Outdated Show resolved Hide resolved
@nohwnd
Copy link
Member Author

nohwnd commented Sep 18, 2024

reverted

@Evangelink Evangelink merged commit 2f24d4c into main Sep 21, 2024
6 checks passed
@Evangelink Evangelink deleted the extract-reflection branch September 21, 2024 14:28
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.

3 participants