-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add test for exception propagation inside loader events #43910
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
Conversation
This test uses the AssemblyResolve handler, but it's arguably a reflection test at the core, so I've put it here. Happy to move it elsewhere as appropriate. It currently passes on Mono due to dotnet#43910 but should not, so I've disabled it for now. Once that issue is resolved, I'll push a fix for this and re-enable the test.
src/libraries/System.Runtime.Extensions/tests/System/AppDomainTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.Extensions/tests/System/AppDomainTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.Extensions/tests/System/AppDomainTests.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to this area: @vitek-karas, @agocke |
985f1a8 to
ac08aa3
Compare
|
This one seems to finally be working as expected. |
|
Still need an approval on this. cc: @vitek-karas |
vitek-karas
left a 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.
@LakshanF can you please also take a look - you've added a similar test recently.
src/libraries/System.Runtime.Extensions/tests/System/AppDomainTests.cs
Outdated
Show resolved
Hide resolved
ad82cd3 to
27d327a
Compare
|
Rebased and took out RemoteExecutor. Hopefully I didn't break something. |
|
Looks to me as a good approach... will take another look once the tests passing. |
4d7e42d to
737c873
Compare
Currently broken on Mono, see #43909