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

[mono] Re-enable tests that call MakeGenericType with non-RuntimeType arguments #64344

Merged
merged 4 commits into from
Jan 27, 2022

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Jan 26, 2022

Partially fixes #32743

The actual fix was in #58014

Some of the re-enabled tests failed on wasm due to #52062, so those have been disabled here.

@ghost
Copy link

ghost commented Jan 26, 2022

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

Issue Details

Fixes #32743

The actual fix was in #58014

Author: lambdageek
Assignees: lambdageek
Labels:

area-Extensions-Logging

Milestone: -

@lambdageek
Copy link
Member Author

Replaces #64195

@radical
Copy link
Member

radical commented Jan 26, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

Some failures on wasm and tvOS. In both cases it looks like the tests probably need to be disabled.

Wasm example. Looks like it's trying to find an assembly in the file system, which won't work:

[18:08:27] fail: [FAIL] Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.WithNullLevel_GeneratorWontFail(argumentList: "EventId = 0, Level = null, Message = \"This is a m"...)
[18:08:27] info: System.ArgumentException : Empty path name is not legal. (Parameter 'path')
[18:08:27] info:    at System.IO.Strategies.FileStreamHelpers.ValidateArguments(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
[18:08:27] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
[18:08:27] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options)
[18:08:27] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, Boolean useAsync)
[18:08:27] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
[18:08:27] info:    at Roslyn.Utilities.StandardFileSystem.OpenFile(String filePath, FileMode mode, FileAccess access, FileShare share)
[18:08:27] info:    at Roslyn.Utilities.CommonCompilerFileSystemExtensions.OpenFileWithNormalizedException(ICommonCompilerFileSystem fileSystem, String filePath, FileMode fileMode, FileAccess fileAccess, FileShare fileShare)
[18:08:27] info:    at Microsoft.CodeAnalysis.MetadataReference.CreateFromFile(String path, MetadataReferenceProperties properties, DocumentationProvider documentation)
[18:08:27] info:    at SourceGenerators.Tests.RoslynTestUtils.CreateTestProject(IEnumerable`1 references, Boolean includeBaseReferences)
[18:08:27] info:    at SourceGenerators.Tests.RoslynTestUtils.RunGenerator(ISourceGenerator generator, IEnumerable`1 references, IEnumerable`1 sources, Boolean includeBaseReferences, CancellationToken cancellationToken)
[18:08:27] info:    at Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.RunGenerator(String code, Boolean wrap, Boolean inNamespace, Boolean includeBaseReferences, Boolean includeLoggingReferences, CancellationToken cancellationToken)
[18:08:27] info:    at Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.WithNullLevel_GeneratorWontFail(String argumentList)
[18:08:27] info: --- End of stack trace from previous location ---

tvOS example. MakeGenericType with non-RuntimeType arguments isn't supported in FullAOT:

System.PlatformNotSupportedException : Operation is not supported on this platform.

Stack trace
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at System.ComponentModel.Composition.Hosting.CompositionServices.AdjustSpecifiedTypeIdentityType(Type specifiedContractType, Type memberType)
   at System.ComponentModel.Composition.Hosting.CompositionServices.AdjustSpecifiedTypeIdentityType(Type specifiedContractType, MemberInfo member)
   at System.ComponentModel.Composition.Hosting.CompositionServices.GetTypeIdentityTypeFromExport(MemberInfo member, ExportAttribute export)
   at System.ComponentModel.Composition.Hosting.CompositionServices.GetContractInfoFromExport(MemberInfo member, ExportAttribute export, Type& typeIdentityType, String& contractName)
   at System.ComponentModel.Composition.AttributedModel.AttributedPartCreationInfo.CreateExportDefinition(MemberInfo member, ExportAttribute exportAttribute)
   at System.ComponentModel.Composition.AttributedModel.AttributedPartCreationInfo.GetExportDefinitions()
   at System.ComponentModel.Composition.AttributedModel.AttributedPartCreationInfo.DiscoverExportsAndImports()
   at System.ComponentModel.Composition.AttributedModel.AttributedPartCreationInfo.GetExports()
   at System.ComponentModel.Composition.ReflectionModel.ReflectionComposablePartDefinition.get_ExportDefinitionsInternal()
   at System.ComponentModel.Composition.ReflectionModel.ReflectionComposablePartDefinition.get_ExportDefinitions()
   at System.ComponentModel.Composition.Hosting.TypeCatalog.CreateIndex()
   at System.Lazy`1[[System.Collections.Generic.IDictionary`2[[System.String, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.List`1[[System.ComponentModel.Composition.Primitives.ComposablePartDefinition, System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1[[System.Collections.Generic.IDictionary`2[[System.String, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.List`1[[System.ComponentModel.Composition.Primitives.ComposablePartDefinition, System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1[[System.Collections.Generic.IDictionary`2[[System.String, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.List`1[[System.ComponentModel.Composition.Primitives.ComposablePartDefinition, System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].CreateValue()
   at System.Lazy`1[[System.Collections.Generic.IDictionary`2[[System.String, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.List`1[[System.ComponentModel.Composition.Primitives.ComposablePartDefinition, System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].get_Value()
   at System.ComponentModel.Composition.Hosting.TypeCatalog.GetCandidateParts(String contractName)
   at System.ComponentMo

@radical
Copy link
Member

radical commented Jan 26, 2022

The wasm ones are failing to get the assembly location with string corelib = Assembly.GetAssembly(typeof(object))!.Location;.
Issue: #52062

`Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorEmitterTests.*`
`Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.*`

.. failing like:

```
[18:09:38] fail: [FAIL] Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.NotPartial
[18:09:38] info: System.ArgumentException : Empty path name is not legal. (Parameter 'path')
[18:09:38] info:    at System.IO.Strategies.FileStreamHelpers.ValidateArguments(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
[18:09:38] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
[18:09:38] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options)
[18:09:38] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, Boolean useAsync)
[18:09:38] info:    at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
[18:09:38] info:    at Roslyn.Utilities.StandardFileSystem.OpenFile(String filePath, FileMode mode, FileAccess access, FileShare share)
[18:09:38] info:    at Roslyn.Utilities.CommonCompilerFileSystemExtensions.OpenFileWithNormalizedException(ICommonCompilerFileSystem fileSystem, String filePath, FileMode fileMode, FileAccess fileAccess, FileShare fileShare)
[18:09:38] info:    at Microsoft.CodeAnalysis.MetadataReference.CreateFromFile(String path, MetadataReferenceProperties properties, DocumentationProvider documentation)
[18:09:38] info:    at SourceGenerators.Tests.RoslynTestUtils.CreateTestProject(IEnumerable`1 references, Boolean includeBaseReferences)
[18:09:38] info:    at SourceGenerators.Tests.RoslynTestUtils.RunGenerator(ISourceGenerator generator, IEnumerable`1 references, IEnumerable`1 sources, Boolean includeBaseReferences, CancellationToken cancellationToken)
[18:09:38] info:    at Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.RunGenerator(String code, Boolean wrap, Boolean inNamespace, Boolean includeBaseReferences, Boolean includeLoggingReferences, CancellationToken cancellationToken)
[18:09:38] info:    at Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorParserTests.NotPartial()
[18:09:38] info: --- End of stack trace from previous location ---
```

.. due to dotnet#52062 .
`System.ComponentModel.Composition.Registration.Tests.RegistrationBuilderTests.ExplicitGenericExportInRegistrationBuilder`
`System.ComponentModel.Composition.Registration.Tests.RegistrationBuilderTests.ExplicitGenericArity2ExportInRegistrationBuilder`
`System.ComponentModel.Composition.Registration.Tests.RegistrationBuilderTests.GenericBaseClassExportInRegistrationBuilder`
`System.ComponentModel.Composition.Registration.Tests.RegistrationBuilderTests.GenericInterfaceExportInRegistrationBuilder`
`System.ComponentModel.Composition.Registration.Tests.RegistrationBuilderTests.ShouldSucceed`

.. failing due to dotnet#32743 .
@radical
Copy link
Member

radical commented Jan 26, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -45,7 +45,7 @@ public DiscoveredCatalog()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/32743", TestRuntimes.Mono)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/32743", TestPlatforms.tvOS)]
Copy link
Member Author

Choose a reason for hiding this comment

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

@radical I don't think this is an ActiveIssue. (or at least 32743 is misleading). I don't think we can support MakeGenericType for non-RuntimeType types in FullAOT (without interpreter fallback).

I think this should be [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoAot))]

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, this is not tvOS specific. Using RuntimeFeature.IsDynamicCode or whatever flag NativeAOT uses would be better

/cc @jkotas

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 guess that's

public static bool IsReflectionEmitSupported => RuntimeFeature.IsDynamicCodeSupported;

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported)]

Which brings up the question: do we run these tests with NativeAOT? I would expect them to fail there, too

Copy link
Member

Choose a reason for hiding this comment

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

Which brings up the question: do we run these tests with NativeAOT?

Not yet. @MichalStrehovsky is working on increasing the test coverage for NativeAOT.

@radical
Copy link
Member

radical commented Jan 27, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

Failures are #64389

@lambdageek lambdageek merged commit d016569 into dotnet:main Jan 27, 2022
@MaximLipnin
Copy link
Contributor

MaximLipnin commented Jan 28, 2022

The re-enabled Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorEmitterTests tests started failing with PNSE on Mono+Windows legs.
E.g.

    Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorEmitterTests.TestEmitter [FAIL]
      System.PlatformNotSupportedException : This API is specific to the way in which Windows handles asynchronous I/O, and is not supported on this platform.
      Stack Trace:
        /_/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolBoundHandle.PlatformNotSupported.cs(24,0): at System.Threading.ThreadPoolBoundHandle.BindHandle(SafeHandle handle)
        /_/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs(184,0): at Microsoft.Win32.SafeHandles.SafeFileHandle.InitThreadPoolBinding()
        /_/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs(54,0): at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/OSFileStreamStrategy.cs(46,0): at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs(18,0): at System.IO.Strategies.AsyncWindowsFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs(35,0): at System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs(24,0): at System.IO.Strategies.FileStreamHelpers.ChooseStrategy(FileStream fileStream, String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs(215,0): at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs(146,0): at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/File.cs(452,0): at System.IO.File.AsyncStreamReader(String path, Encoding encoding)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/File.cs(479,0): at System.IO.File.InternalReadAllTextAsync(String path, Encoding encoding, CancellationToken cancellationToken)
        /_/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs(23,0): at Microsoft.Extensions.Logging.Generators.Tests.LoggerMessageGeneratorEmitterTests.TestEmitter()
        --- End of stack trace from previous location ---

Perhaps, it's being tracked in #34582

MaximLipnin added a commit that referenced this pull request Jan 28, 2022
…LoggerMessageGeneratorEmitterTests with ActiveIssue (#64436)

Addresses #64344 (comment)

... and skips System.Reflection.Emit.Tests.ModuleBuilderDefineInitializedData.DefineInitializedData_EnsureAlignmentIsMinimumNeededForUseOfCreateSpan test which requires reflection emit and fails on tvOS arm64 leg.
@ghost ghost locked as resolved and limited conversation to collaborators Feb 27, 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.

System.RuntimeType.MakeGenericType throws NotImplementedException
7 participants