[QUIC] Fix test for debug/release version of a library#93449
[QUIC] Fix test for debug/release version of a library#93449ManickaP merged 1 commit intodotnet:mainfrom
Conversation
ghost
added
the
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThe originally used preexisting platform detection is for runtime not libraries. Fixes #93404
|
There was a problem hiding this comment.
isn't this the same as s_isReleaseRuntime and s_isDebugRuntime ?
AssemblyConfigurationEquals checks typeof(string).Assembly so I think an easier solution would be to add an overload that takes the assembly there instead of IsDebuggable(Assembly assembly)
There was a problem hiding this comment.
Oh no, those are remnants, they shouldn't be here. The two bellow are important.
There was a problem hiding this comment.
ok but why do you need to check DebuggableAttribute instead of adding an overload to the existing AssemblyConfigurationEquals?
There was a problem hiding this comment.
As far as I can see in my tests, it returns the configuration of runtime, not the library and the attribute is not present for System.Net.Quic.dll:
Console.WriteLine(typeof(QuicConnection).Assembly.GetCustomAttribute<AssemblyConfigurationAttribute>() is null); // True
There was a problem hiding this comment.
ah I see, looks like we disable generating AssemblyConfiguration attributes for all libraries:
runtime/eng/versioning.targets
Lines 3 to 4 in b9acf73
ManickaP
force-pushed
the
mapichov/quic-fix-test
branch
from
5cf81c7 to
06a85f0
Compare
|
Failures are #92423 |
| if (PlatformDetection.IsReleaseLibrary(typeof(QuicConnection).Assembly)) | ||
| { | ||
| throw new SkipTestException("Retrieving SSL secrets is not supported in Release mode."); |
There was a problem hiding this comment.
This is nitpicking, but since there's also 'checked' build possibility, I feel it might be better to check !IsDebugLibrary instead of IsReleaseLibrary (I see that implementation-wise this is the same, but it would make a bit more sense to me)
| if (PlatformDetection.IsReleaseLibrary(typeof(QuicConnection).Assembly)) | |
| { | |
| throw new SkipTestException("Retrieving SSL secrets is not supported in Release mode."); | |
| if (!PlatformDetection.IsDebugLibrary(typeof(QuicConnection).Assembly)) | |
| { | |
| throw new SkipTestException("Retrieving SSL secrets is supported only in Debug mode."); |
There was a problem hiding this comment.
Baah, I spent too much time writing that 😂 you can disregard since this is merged already
There was a problem hiding this comment.
There's no "checked" version of library, that applies only to CLR.
4 participants
The originally used preexisting platform detection is for runtime not libraries.
Fixes #93404