DataGridView flaky tests. by kirsan31 · Pull Request #7031 · dotnet/winforms

kirsan31 · 2022-04-16T11:33:48Z

Attempt to investigate DGV flaky tests. #6926 #6739

Both tests fail due to extra invalidate. To investigate further we need to understand 2 things:

Couse of invalidate - call stack.
Input parameters pattern: rowHeadersVisible: True / columnHeadersVisible: True

Fails by run №:

11: Windows_x86-xunit
System.Windows.Forms.Tests.ToolStripItemTests.ToolStripItem_ImageKey_ShouldSerializeValueWithOwnerWithImageList_Success
27: Windows_arm64-xunit
System.Windows.Forms.Tests.DataGridViewTests.DataGridView_DefaultCellStyles_Rendering
29: Windows_x64-xunit
System.Windows.Forms.Tests.DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected. Got it!

The problem is in random cursor position during tests.

Microsoft Reviewers: Open in CodeFlow

RussKie · 2022-04-17T04:05:27Z

/azp run

azure-pipelines · 2022-04-17T04:05:40Z

Azure Pipelines successfully started running 2 pipeline(s).

kirsan31 · 2022-04-17T07:49:05Z

Ups 🙄
I will amend + push then...

azure-pipelines · 2022-04-17T07:49:11Z

Commenter does not have sufficient privileges for PR 7031 in repo dotnet/winforms

RussKie · 2022-04-17T10:09:00Z

"azp run" only works for those with write access. To rerun the build pipeline you can either (f-)push or close-reopen.

kirsan31 · 2022-04-17T14:41:41Z

On 11 run we found another one flaky test or may be something new?

27 : https://dev.azure.com/dnceng/public/_build/results?buildId=1723543&view=ms.vss-test-web.build-test-results-tab&runId=46761178&resultId=106555&paneView=debug

Tanya-Solyanik · 2022-04-18T04:51:43Z

+            // Flaky tests, see: https://github.com/dotnet/winforms/issues/6739
+            control.Invalidated += (sender, e) =>
+            {
+                Assert.True(++invalidatedCallCount <= expectedInvalidatedCallCount,


You mentioned in the description that you wanted to print out Environment.StackTrace, no?

We will see it if assert inside event handler fail. For example:

System.Windows.Forms.Tests.DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected(rowHeadersWidthSizeMode: EnableResizing, rowHeadersVisible: False, autoSize: True, value: 4, expectedValue: 4, expectedInvalidatedCallCount: 1) [FAIL] Input: EnableResizing, False, True, 4 Expected: True Actual: False Stack Trace: ...\winforms\src\System.Windows.Forms\tests\UnitTests\System\Windows\Forms\DataGridViewTests.cs(1127,0): at System.Windows.Forms.Tests.DataGridViewTests.<>c__DisplayClass40_0.<DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected>b__0(Object sender, InvalidateEventArgs e) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs(8212,0): at System.Windows.Forms.Control.OnInvalidated(InvalidateEventArgs e) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs(7091,0): at System.Windows.Forms.Control.NotifyInvalidate(Rectangle invalidatedArea) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs(6432,0): at System.Windows.Forms.Control.Invalidate(Rectangle rc, Boolean invalidateChildren) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs(6396,0): at System.Windows.Forms.Control.Invalidate(Rectangle rc) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\DataGridView.Methods.cs(10109,0): at System.Windows.Forms.DataGridView.InvalidateInside() ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\DataGridView.cs(3666,0): at System.Windows.Forms.DataGridView.set_RowHeadersWidthInternal(Int32 value) ...\winforms\src\System.Windows.Forms\src\System\Windows\Forms\DataGridView.cs(3650,0): at System.Windows.Forms.DataGridView.set_RowHeadersWidth(Int32 value) ...\winforms\src\System.Windows.Forms\tests\UnitTests\System\Windows\Forms\DataGridViewTests.cs(1138,0): at System.Windows.Forms.Tests.DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected(DataGridViewRowHeadersWidthSizeMode rowHeadersWidthSizeMode, Boolean rowHeadersVisible, Boolean autoSize, Int32 value, Int32 expectedValue, Int32 expectedInvalidatedCallCount) at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor) at System.Reflection.RuntimeMethodInfo.InvokeNonEmitUnsafe(Object obj, IntPtr* arguments, Span`1 argsForTemporaryMonoSupport, BindingFlags invokeAttr)

P.s. I noticed that the input parameters were always there (they were just cut off on the page and it was hard to see them). 😳
So, Input parameters pattern are: rowHeadersVisible: True and columnHeadersVisible: True ...

kirsan31 · 2022-04-18T11:49:42Z

Extra invalidate call stack:

at System.Windows.Forms.Tests.DataGridViewTests.<>c__DisplayClass40_0.<DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected>b__0(Object sender, InvalidateEventArgs e) in /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataGridViewTests.cs:line 1127
   at System.Windows.Forms.Control.OnInvalidated(InvalidateEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 8212
   at System.Windows.Forms.Control.NotifyInvalidate(Rectangle invalidatedArea) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 7091
   at System.Windows.Forms.Control.Invalidate(Rectangle rc, Boolean invalidateChildren) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 6432
   at System.Windows.Forms.Control.Invalidate(Rectangle rc) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 6396
   at System.Windows.Forms.DataGridView.set_TopLeftHeaderCell(DataGridViewHeaderCell value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 4364
   at System.Windows.Forms.DataGridView.get_TopLeftHeaderCell() in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 4337
   at System.Windows.Forms.DataGridView.GetCellInternal(Int32 columnIndex, Int32 rowIndex) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 7274
   at System.Windows.Forms.DataGridView.OnCellMouseEnter(DataGridViewCellEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 13019
   at System.Windows.Forms.DataGridView.UpdateMouseEnteredCell(HitTestInfo hti, MouseEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 29638
   at System.Windows.Forms.DataGridView.OnRowHeadersWidthChanged(EventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 18823
   at System.Windows.Forms.DataGridView.set_RowHeadersWidthInternal(Int32 value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 3677
   at System.Windows.Forms.DataGridView.set_RowHeadersWidth(Int32 value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 3650
   at System.Windows.Forms.Tests.DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected(DataGridViewRowHeadersWidthSizeMode rowHeadersWidthSizeMode, Boolean rowHeadersVisible, Boolean autoSize, Int32 value, Int32 expectedValue, Int32 expectedInvalidatedCallCount) in /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataGridViewTests.cs:line 1138
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.RuntimeMethodInfo.InvokeNonEmitUnsafe(Object obj, IntPtr* arguments, Span`1 argsForTemporaryMonoSupport, BindingFlags invokeAttr)

It's obvious that mouse pointer involved here 😁 I will update after investigation...

--- UPD ---

Yes, the problem is in random mouse position.
In UpdateMouseEnteredCell we get MousePosition:

Point ptMouse = PointToClient(Control.MousePosition);

then in HitTest we check if pointer over some DGV element (+ changed) we will call OnCellMouseEnter - extra invalidate...

I think similar problem may be with every control in current tests. We need to take strict control over cursor position in such situations. Unfortunately I don't know your test infra (relation between screen and client coordinates) to solve this by myself...

Tanya-Solyanik · 2022-04-21T21:02:54Z

            control.OnColumnHeadersHeightChanged(eventArgs);
            Assert.Equal(1, callCount);
            Assert.True(control.IsHandleCreated);
-            Assert.Equal(0, invalidatedCallCount);


You could replace strict comparison with (invalidatedCallCount < 2). Or you could replace look up the call stack inside the Invalidated event handler and don't increment the counter for call stacks with Mouse entered the cell and invalidated it method(s).

Obviously, these tests were written without taking into account the mouse pointer, i.e. it shouldn't be above the control.

You could replace strict comparison with (invalidatedCallCount < 2).

This will eliminate the original test in case of mouse not above the cell and not strictly right in case of mouse above the cell (there can be 2 invalidates in case of column test and even 3 (or maybe more?) in case of row test).

Or you could replace call stack inside the Invalidated event handler and don't increment the counter for call stacks with Mouse entered the cell and invalidated it method(s).

This will eliminate the original test in case of mouse above the cell.

And I think that overall this is not right to have unpredictable random variable (mouse pointer) in tests...
We have lots (about 4k) of invalidate count checks in WinForms tests, and all of them potentially can be affected by this problem :(
So, the best solution will be to have control of mouse pointer location. And explicitly set it out of the tested control for such invalidate tests. But I don't know how difficult it is to achieve this.

Would be great to Moq the Mouse of cause.

We can't mock the system-wide singletons like mouse or the clipboard.

What we can do in the tests though is to set the mouse to one of the corners. Given that some of the tests place controls at (0, 0), it probably not the corner to take. I'd put the mouse at the (Screen.PrimaryScreen.WorkingArea.Width, Screen.PrimaryScreen.WorkingArea.Height) at the start of each test. I'm pretty sure we can do it here:

winforms/src/System.Windows.Forms/tests/TestUtilities/ThreadExceptionFixture.cs

Lines 12 to 17 in a9dbf1b

public ThreadExceptionFixture()

{

Application.EnableVisualStyles();

Application.ThreadException += OnThreadException;

}

I'd also add an assertion here to verify the mouse is still there at the end of the test to weed out any tests that involve the mouse (and have those moved to the UI integration tests project):

winforms/src/System.Windows.Forms/tests/TestUtilities/ThreadExceptionFixture.cs

Lines 19 to 22 in a9dbf1b

public virtual void Dispose()

{

Application.ThreadException -= OnThreadException;

}

💭 We should probably rename the fixture too, since it's doing more than dealing with the ThreadException event.

WinFormsTestFixture? :)

…me tests.

kirsan31 · 2022-07-10T13:28:25Z

@RussKie once again sorry for the long delay :(

I'd put the mouse at the (Screen.PrimaryScreen.WorkingArea.Width, Screen.PrimaryScreen.WorkingArea.Height) at the start of each test. I'm pretty sure we can do it here:

Done.

I'd also add an assertion here to verify the mouse is still there at the end of the test to weed out any tests that involve the mouse (and have those moved to the UI integration tests project):

I tried this, but a huge number of tests failed due to the fact that the cursor moved by one pixel (even if I held the mouse over the table):

We should probably rename the fixture too, since it's doing more than dealing with the ThreadException event.

Name suggestion?

P.s.
I have several (quantity may varies from run to run) tests failed on my pc with clean main build. Most of them are parsing / formatting (number, datetime etc).
Windows 10 21H1
V.S. (17.2.5) language is English. Windows language is English. Region and regional format in windows settings - Ru.
.Net SDK 7.0.100-preview.5.22307.18

Details: FailedTests.xlsx

kirsan31 · 2022-07-19T08:23:14Z

/cc @RussKie about failing tests. The bottom of my previous post.

RussKie · 2022-07-19T09:33:08Z

Ah, how cool! More region-specific failed tests! :) Can you please try applying UseDefaultXunitCultureAttribute and see if it fixes the issues? See #6985.

@vladimir-krestov do you or anyone from your team observe similar issues with tests failing due to the regional settings?

RussKie · 2022-07-19T09:42:16Z

I'd also add an assertion here to verify the mouse is still there at the end of the test to weed out any tests that involve the mouse (and have those moved to the UI integration tests project):

I tried this, but a huge number of tests failed due to the fact that the cursor moved by one pixel (even if I held the mouse over the table):

That's bizarre. Is mouse repositioned before each test?
Also I see only a single ResxDataNode_ResXFileRefConstructor whereas your screenshot suggests a theory. Did you tweak locally?

RussKie · 2022-07-19T09:47:01Z

/azp run

azure-pipelines · 2022-07-19T09:47:16Z

Azure Pipelines successfully started running 2 pipeline(s).

RussKie · 2022-07-19T09:49:18Z

        public virtual void Dispose()
        {
            Application.ThreadException -= OnThreadException;
+            //Xunit.Assert.Equal(new Drawing.Point(Screen.PrimaryScreen.WorkingArea.Width, Screen.PrimaryScreen.WorkingArea.Height), Cursor.Position);


This should go, but since there is more work in this area is lined up I'm going to take it as is with the promise that this will get fixed in a follow up PR.

kirsan31 · 2022-07-19T10:02:58Z

That's bizarre. Is mouse repositioned before each test?
Also I see only a single ResxDataNode_ResXFileRefConstructor whereas your screenshot suggests a theory. Did you tweak locally?

Yes very strange :(
I chose ResxDataNode_ResXFileRefConstructor on purpose so that there is only one test and it is obvious that nothing can move the pointer.
Nice catch about 2 results instead of one 😳 No, there were no local changes - VS bug? I will try to investigate...

RussKie · 2022-07-19T10:53:59Z

Just a random crazy thought, what if we hid the cursor for the duration of a test? It wouldn't cause any invalidation events, would it?

winforms/src/System.Windows.Forms/src/System/Windows/Forms/Cursor.cs

Line 388 in 7c09c53

public static void Hide() => User32.ShowCursor(BOOL.FALSE);

kirsan31 · 2022-08-03T17:25:00Z

@RussKie
I think our fix with mouse positioning is not working 😥

https://dev.azure.com/dnceng/public/_build/results?buildId=1920544&view=ms.vss-test-web.build-test-results-tab&runId=49742938&resultId=104397&paneView=debug (need to click exit full screen mode button)

at System.Windows.Forms.Tests.DataGridViewTests.<>c__DisplayClass40_0.<DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected>b__0(Object sender, InvalidateEventArgs e) in /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataGridViewTests.cs:line 1126
at System.Windows.Forms.Control.OnInvalidated(InvalidateEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 8263
at System.Windows.Forms.Control.NotifyInvalidate(Rectangle invalidatedArea) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 7142
at System.Windows.Forms.Control.Invalidate(Rectangle rc, Boolean invalidateChildren) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 6472
at System.Windows.Forms.Control.Invalidate(Rectangle rc) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 6436
at System.Windows.Forms.DataGridView.set_TopLeftHeaderCell(DataGridViewHeaderCell value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 4364
at System.Windows.Forms.DataGridView.get_TopLeftHeaderCell() in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 4337
at System.Windows.Forms.DataGridView.GetCellInternal(Int32 columnIndex, Int32 rowIndex) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 7275
at System.Windows.Forms.DataGridView.OnCellMouseEnter(DataGridViewCellEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 13024
at System.Windows.Forms.DataGridView.UpdateMouseEnteredCell(HitTestInfo hti, MouseEventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 29642
at System.Windows.Forms.DataGridView.OnRowHeadersWidthChanged(EventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.Methods.cs:line 18828
at System.Windows.Forms.DataGridView.set_RowHeadersWidthInternal(Int32 value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 3677
at System.Windows.Forms.DataGridView.set_RowHeadersWidth(Int32 value) in /_/src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs:line 3650
at System.Windows.Forms.Tests.DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected(DataGridViewRowHeadersWidthSizeMode rowHeadersWidthSizeMode, Boolean rowHeadersVisible, Boolean autoSize, Int32 value, Int32 expectedValue, Int32 expectedInvalidatedCallCount) in /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataGridViewTests.cs:line 1137
at InvokeStub_DataGridViewTests.DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected(Object, Object, IntPtr*)
at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

The main question - why? 🤷‍♂️ And then we can try your random crazy thought...

But I have very limited time those days and on next week go to vocation 🙄

kirsan31 requested a review from a team as a code owner April 16, 2022 11:33

ghost assigned kirsan31 Apr 16, 2022

This comment was marked as resolved.

Sign in to view

kirsan31 force-pushed the DGV_FlakyTests branch 2 times, most recently from c60d213 to 8623e37 Compare April 17, 2022 09:56

kirsan31 force-pushed the DGV_FlakyTests branch 7 times, most recently from 760c344 to 3b4db3b Compare April 17, 2022 14:09

kirsan31 force-pushed the DGV_FlakyTests branch 11 times, most recently from 6e86fa4 to 217fda9 Compare April 17, 2022 21:11

Tanya-Solyanik reviewed Apr 18, 2022

View reviewed changes

kirsan31 force-pushed the DGV_FlakyTests branch 2 times, most recently from 0d7c54b to d00de1e Compare April 18, 2022 10:31

This was referenced Apr 18, 2022

Flaky test DataGridView_RowHeadersWidth_SetWithHandle_GetReturnsExpected #6739

Closed

Flaky test DataGridView_OnColumnHeadersHeightChanged_InvokeWithHandle_CallsColumnHeadersHeightChanged #6926

Closed

Tanya-Solyanik reviewed Apr 21, 2022

View reviewed changes

Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Apr 22, 2022

ghost removed the waiting-author-feedback The team requires more information from the author label Apr 24, 2022

RussKie added the waiting-author-feedback The team requires more information from the author label May 6, 2022

Moved the cursor to the bottom right corner to avoid its effect on so…

9daa9bb

…me tests.

kirsan31 force-pushed the DGV_FlakyTests branch from d00de1e to 9daa9bb Compare July 10, 2022 12:16

ghost removed the waiting-author-feedback The team requires more information from the author label Jul 10, 2022

runfoapp Bot mentioned this pull request Jul 11, 2022

Flaky System.Windows.Forms.UITests.ButtonTests.Button_Anchor_ResizeOnWindowSizeWiderAsync #7297

Closed

kirsan31 changed the title ~~Attempt to investigate DGV flaky tests.~~ DataGridView flaky tests. Jul 19, 2022

kirsan31 mentioned this pull request Jul 19, 2022

Workaround for empty finalizer in ApplicationContext. #7431

Merged

RussKie approved these changes Jul 19, 2022

View reviewed changes

RussKie enabled auto-merge (squash) July 19, 2022 09:47

RussKie reviewed Jul 19, 2022

View reviewed changes

RussKie merged commit 82d6c02 into dotnet:main Jul 19, 2022

ghost added this to the 7.0 RC1 milestone Jul 19, 2022

ghost locked as resolved and limited conversation to collaborators Sep 2, 2022

	public ThreadExceptionFixture()
	{
	Application.EnableVisualStyles();

	Application.ThreadException += OnThreadException;
	}

	public virtual void Dispose()
	{
	Application.ThreadException -= OnThreadException;
	}

Conversation

kirsan31 commented Apr 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

RussKie commented Apr 17, 2022

Uh oh!

azure-pipelines Bot commented Apr 17, 2022

Uh oh!

kirsan31 commented Apr 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Apr 17, 2022

Uh oh!

This comment was marked as resolved.

RussKie commented Apr 17, 2022 via email • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirsan31 commented Apr 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirsan31 Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirsan31 commented Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirsan31 commented Jul 10, 2022

Uh oh!

kirsan31 commented Jul 19, 2022

Uh oh!

RussKie commented Jul 19, 2022

Uh oh!

RussKie commented Jul 19, 2022

Uh oh!

RussKie commented Jul 19, 2022

Uh oh!

azure-pipelines Bot commented Jul 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirsan31 commented Jul 19, 2022

Uh oh!

RussKie commented Jul 19, 2022

Uh oh!

kirsan31 commented Aug 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kirsan31 commented Apr 16, 2022 •

edited

Loading

kirsan31 commented Apr 17, 2022 •

edited

Loading

RussKie commented Apr 17, 2022 via email •

edited by ghost

Loading

kirsan31 commented Apr 17, 2022 •

edited

Loading

kirsan31 Apr 18, 2022 •

edited

Loading

kirsan31 commented Apr 18, 2022 •

edited

Loading