Integration Tests which make use of `ControlTestBase` can erroneously pass when an exception is present

[elachlan]

Add the following test to System.Windows.Forms.UI.IntegrationTests. It should fail as an exception is thrown. But it passes.

[WinFormsFact]
public async Task Special_Test()
{
    using Form form = new();
    form.Shown += (object? sender, EventArgs e) => { throw new Exception("test"); };
    await RunFormWithoutControlAsync(() => form, async (form) =>
    {
        await MoveMouseAsync(form, new Point(0, 0), false);
        form.Close();
    });
}

[dmitrii-drobotov]

It's probably have to do with exception being thrown in event handler, I remember a similar case here: https://github.com/dotnet/winforms/pull/8283#issuecomment-1328177999. You can also reproduce it with a simple unit test, for example

[WinFormsFact]
public void Special_Test()
{
    using Form form = new();
    form.Shown += (object? sender, EventArgs e) => { throw new Exception("test"); };
    form.ShowDialog();
}

So I would say this is expected behavior.

[elachlan]

Thanks that make sense. If I added a try catch it doesn't actually help for errors that are caused inside the winforms code. I tried to avoid this by using SetUnhandledExceptionMode but it didn't work.

@AArnott this is the issue I was talking about. Could Xunit.StaFact have an attribute that sets the UnhandledExceptionMode per test or something along those lines? When I tried I received an exception claiming it had to be done before any controls had been created.

[AArnott]

It sounds like WinForms deliberately swallows these exceptions. In general, a test can only fail if the exception can be detected, and a swallowed exception isn't something that should fail a test because swallowing exceptions is intentional in some code and shouldn't force test failures. Uncaught exceptions always crash a process, no matter what thread they are on. Test runners generally catch exceptions that escape the test method and report it as a test failure to avoid a process crash. GUI frameworks sometimes have no catch around event handlers, so an exception thrown from an event handler may crash the process. async void methods that throw almost always crash the process.

Now, WinForms' ThreadException handler isn't one I'm familiar with. If that would provide a way to 'catch' exceptions that would otherwise be swallowed, that could potentially be used as a hook to cause a test failure, although we'd need to know which test to associate the exception with. It would presumably preclude running tests in parallel too, but WinFormsFact may already have forced all tests to run sequentially, I don't recall. Apparently throwing from these handlers doesn't break the winforms app itself, so should it fail a unit test?

All this is to say... I don't know what we should or can do in xunit.stafact to help catch these failures. If you understand the nuances of this exception handler more, maybe we should continue the discussion in the xunit.stafact repo.

[elachlan]

Thank you, your input is greatly appreciated. I'll discuss it a bit more here with the team. I think we can use the Application.SetUnhandledExceptionMode(https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.application.setunhandledexceptionmode?view=windowsdesktop-7.0) at the moment it is showing the unhandled exception message box. Whereas in a test it should fail, unless we are testing the unhandled exception message box.

[Tanya-Solyanik]

@elachlan - had you tried setting a thread exception handler on the worker thread? - https://github.com/dotnet/winforms/blob/7338e29de01f9415208dfbfbfa89448c53812e08/src/System.Windows.Forms/src/System/Windows/Forms/Application.ThreadContext.cs#L862

[elachlan]

@Tanya-Solyanik I hadn't tried it. That would make sense. I figure for a winforms test, it might make sense to have this automatically handled, so failures are detected no matter the test.

There are currently tests in winforms which I think are failing which don't capture these errors as test failures.

[Tanya-Solyanik]

@elachlan - we can start by implementing a pattern in this repo and then moving it down the stack.

[elachlan]

Related: #8657