Winforms: Missing Control.Dispose calls in unit tests

[weltkante]

.NET Core Version:
github master

Have you experienced this same bug with .NET Framework?:
n/a

Problem description:
Disposing controls in unit tests is required because the usage of multiple UI threads. If the control has not been GC'ed by the time the UI thread terminates this can lead to hangs.

Related to #3348 but this is short-term bugfixing while the other issue is long-term reliability to detect regressions in unit test quality.

Following call sites do not dispose their controls:

• [ ] AxHostTests.AxHost_EndInit_InvokeWithParent_CreatesControl
• [x] ComboBoxTests.CtrlBackspaceDeletesSelection
• [x] ComboBoxTests.CtrlBackspaceRepeatedTextChanged
• [x] ComboBoxTests.CtrlBackspaceTextChanged
• [x] ComboBoxTests.CtrlBackspaceTextRemainsEmpty
• [ ] ControlControlAccessibleObject.Handle_Set_TestData
• [ ] ControlControlCollectionTests.ControlCollection_Add _DifferentThreadValueControl_ThrowsArgumentException
• [ ] ControlControlCollectionTests.ControlCollection_Add _DifferentThreadValueOwner_ThrowsArgumentException
• [ ] ControlTests.AccessibilityObject_CustomCreateAccessibilityInstance_TestData
• [ ] ControlTests.Control_Dispose_InvokeNotDisposingWithHandle_Success
• [x] ListViewTests.ListView_BackColor_SetWithHandle_GetReturnsExpected
• [x] ListViewTests.ListView_DoubleBuffered_SetWithHandle_GetReturnsExpected
• [x] ListViewTests.ListView_FocusedItem_SetChildWithHandle_GetReturnsExpected
• [x] ListViewTests.ListView_FocusedItem_SetWithHandle_GetReturnsExpected
• [x] ListViewTests.ListView_ForeColor_SetWithHandle_GetReturnsExpected
• [x] PictureBoxTests.PictureBox_Enabled_SetWithHandle_GetReturnsExpected
• [x] PropertyGridInternal.Tests.GridViewListBoxAccessibleObjectTests
  .GridViewListBoxAccessibleObject_DoesNotThrowException_OnFocus
• [ ] ScreenTests.FromHandle_TestData
• [ ] ToolStripRenderEventArgsTests.ConnectedArea_Empty_TestData
• [x] ToolTipTests.ToolTip_Show_InvokeStringIWin32WindowControlWindow_Nop
• [x] ToolTipTests.ToolTip_Show_InvokeStringIWin32WindowIntControlWindow_Nop
• [ ] WebBrowserTests.WebBrowserDispose_InvokeNotDisposing_Success

Checked entries look like they are straightforward to fix, unchecked ones require refactoring of the tests or deeper investigation.

Further entries discovered independently (see issue discussion below)

• [x] ErrorProviderAccessibleObjectTests constructor, test class needs to be disposable
• [ ] ColorDialog never disposes its ColorUI - a cached popup control

Expected behavior:
All unit tests dispose their controls.

Minimal repro:
In the absence of #3348 you can set a conditional breakpoint (or write logging code) in NativeWindow.ForceExitMessageLoop and check for handle != IntPtr.Zero && ownedHandle to detect problematic cases. To identify who created the control capture a stack trace in the NativeWindow constructor (call new StackTrace() and store it as a member).

[weltkante]

/cc @hughbe in case this is of interest, /cc @RussKie FYI

I'm not starting a PR since I only know how to fix the trivial cases (marked above) - though if you want a partial PR for this issue I can do that.

[weltkante]

While looking at #3391 I noticed ErrorProviderAccessibleObjectTests test class constructor creates controls which are apparently not disposed. Fix is to implement IDisposable on the test class.

I'm surprised this doesn't appear in my list above, seems the GC never ran during my experimenting, maybe the test executed late and process terminated before GC collected these.

[RussKie]

We're stress testing the runtime and the tests we've (👀 @ @hughbe) written. Whilst it is somewhat frustrating, I think, overall it has been a very good learning for all of us. I expect to find more bugs, but I think we're getting better at it :)
To top it off we found few genuine bugs too.

[RussKie]

if you want a partial PR for this issue I can do that.

I'm happy with partial fixes, less to worry about :)

[weltkante]

PR for the simple cases is up.

Also found a control leak outside unit tests, ColorDialog caches its ColorUI popup but never disposes it, and the class is not disposable either.

Currently this is a liability for unit testing, disposing of the dialog is only possible via reflection. I doubt it'll cause issues in the field unless someone builds a multithreaded designer for some reason which shows popups on a seperate UI thread.