-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] Test failed on Helix: System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException #31650
Comments
We seem to throw the correct exception, but the test is failing because the error message itself does not contain the filename and instead has it specified below. This seems unnecessarily strict (though our message isn't great), but if we really want the path included in both places it should be an easy enough fix. Sample program: using System;
using System.Reflection;
using System.IO;
namespace HelloWorld
{
class Program
{
static void Main(string[] args)
{
string rootedPath = Path.GetFullPath(Guid.NewGuid().ToString("N"));
Console.WriteLine(rootedPath);
Assembly.LoadFile(rootedPath);
}
}
} Mono/CoreCLR output:
|
I think I made the CoreCLR change -- where we have the file, we put it in the message, and where it's in the message, ToString doesn't bother repeating it on the next line. The reasoning behind this (beyond compact is nice) is that it's common to see an exception stringized into a log, and sometimes logs contain output from several concurrent activities that can get interleaved (eg., build logs). Having the value on the same line as the message keeps them together. I did something similar for some others, eg the argument for ArgumentException. |
Having said that, I can't find such a change, but that's the philosophy. (It's good to have the file name in FileName, also in the message, and ToString should only show the latter.) |
Makes sense to me. It's an easy enough change and I don't see any harm in it, so if you want to leave the tests I'll add it to our messages (and maybe clean them up a bit in the process - I'm not a big fan of just "Invalid Image" everywhere). |
Fixes dotnet/runtime#31649 Fixes dotnet/runtime#31650 **Testing**: Reproduced Error via `<Runtime Repo Root>/build.sh /p:RuntimeFlavor=mono` Removing `[ActiveIssue("dotnet/runtime#31650", TestRuntimes.Mono)]` in AssemblyTests.cs Running `dotnet msbuild /t:RebuildAndTest /p:XunitMethodName=System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException` Fixing issue resulted in no errors.
Fixes dotnet/runtime#31649 Fixes dotnet/runtime#31650 **Testing**: Reproduced Error via `<Runtime Repo Root>/build.sh /p:RuntimeFlavor=mono` Removing `[ActiveIssue("dotnet/runtime#31650", TestRuntimes.Mono)]` in AssemblyTests.cs Running `dotnet msbuild /t:RebuildAndTest /p:XunitMethodName=System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException` Fixing issue resulted in no errors.
Fixes dotnet/runtime#31649 Fixes dotnet/runtime#31650 **Testing**: Reproduced Error via `<Runtime Repo Root>/build.sh /p:RuntimeFlavor=mono` Removing `[ActiveIssue("dotnet/runtime#31650", TestRuntimes.Mono)]` in AssemblyTests.cs Running `dotnet msbuild /t:RebuildAndTest /p:XunitMethodName=System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException` Fixing issue resulted in no errors. Co-authored-by: mdh1418 <mdh1418@users.noreply.github.com>
System.Reflection.Tests.AssemblyTests.LoadFile_NoSuchPath_ThrowsFileNotFoundException
fails on Helix. It will be skipped with ActiveIssue in #2087.The text was updated successfully, but these errors were encountered: