-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix compiler warnings in test wrapper generation #19172
Fix compiler warnings in test wrapper generation #19172
Conversation
There were lots of warnings like the following: ``` Loader.classloader.XUnitWrapper.cs(96333,37): warning CS0168: The variable 'ex' is declared but never used ```
@BruceForstall I am in middle of fixing these warnings and many others as well in PR #19109 and fix will be more comprehensive. If you woant to proceed with this PR let me know. |
@dotnet/dnceng "No space left on device" https://ci.dot.net/job/dotnet_coreclr/job/master/job/x64_checked_ubuntu_corefx_innerloop_prtest/597/
|
@4creators I can wait for your change. Is it going to happen soon? It looks like there's some controversy, and you haven't touched it in a few days. |
This particular machine had a tmp drive filled with core dumps. I think corefx's. I've deleted it. |
@@ -256,7 +256,7 @@ namespace $([System.String]::Copy($(Category)).Replace(".","_").Replace("\",""). | |||
{ | |||
sErrorText = System.IO.File.ReadAllText(errorFile)%3B | |||
} | |||
catch(Exception ex) | |||
catch(Exception) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feedback on #19109 was to print the exception details here. #19109 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please print the exception itself. It costs very little but offers so much when reading the logs. Thanks @tannergooding !
@BruceForstall I will push new commits today if it's OK |
@BruceForstall This can be closed as #19180 is merged. |
There were lots of warnings like the following: