dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.41k stars 982 forks source link

Flaky Clipboard Unit Tests #3456

Closed weltkante closed 3 months ago

weltkante commented 4 years ago

.NET Core Version: master

Have you experienced this same bug with .NET Framework?: no

Problem description: Noticed as a flaky test:

System.Windows.Forms.Tests.TextBoxBaseTests.TextBoxBase_Paste_InvokeNotEmpty_Success [FAIL] Assert.Equal() Failure  (pos 1) Expected: abc Actual: a  (pos 1) Stack Trace: F:\workspace_work\1\s\src\System.Windows.Forms\tests\UnitTests\TextBoxBaseTests.cs(5957,0): at System.Windows.Forms.Tests.TextBoxBaseTests.TextBoxBase_Paste_InvokeNotEmpty_Success()

Test seems to assume clipboard is empty/nonfunctional (there is at least one more test along these, should review for more clipboard-dependent tests)

Also any clipboard tests need to be [Collection("Sequential")] to avoid overlapping with other clipboard tests. Is it worth making the whole test class sequential just due to that single test? should the test be moved?

Expected behavior: Clipboard based tests initialize clipboard to be in known state and are marked for sequential execution

Minimal repro: none, flaky test

RussKie commented 4 years ago

I wonder if we could employ Trait decorations, e.g. something like:

        [Trait(Category.Synchronous, Category.Clipboard)] // arbitrary strings represented by consts
        [WinFormsFact]
        public void TextBoxBase_Paste_InvokeNotEmpty_Success()
        {
            // ...
        }

...and then update our CI configs to run tests separately: https://github.com/dotnet/winforms/blob/0f90af477f1aa7da73828413b5d335adb45950ed/eng/ci.yml#L98-L107

        # Run Unit Tests
        # Tests are run with /m:1 to work around https://github.com/tonerdo/coverlet/issues/364
        - script: eng\cibuild.cmd
            -test
            -configuration $(_BuildConfig)
            $(_OfficialBuildIdArgs)
            /p:Coverage=$(_Coverage)
            /bl:$(BUILD.SOURCESDIRECTORY)\artifacts\log\$(_BuildConfig)\Test.binlog
            /m:1
           -trait "Synchronous=Clipboard"
          displayName: Run Unit Tests (Synchronous)

        - script: eng\cibuild.cmd
            -test
            -configuration $(_BuildConfig)
            $(_OfficialBuildIdArgs)
            /p:Coverage=$(_Coverage)
            /bl:$(BUILD.SOURCESDIRECTORY)\artifacts\log\$(_BuildConfig)\Test.binlog
            /m:1
           -notrait "Synchronous=Clipboard"
          displayName: Run Unit Tests

Got some inspiration from http://www.brendanconnolly.net/organizing-tests-with-xunit-traits/

weltkante commented 4 years ago

Failed test Clipboard_SetText_InvokeStringTextDataFormat_GetReturnsExpected in CI run of PR #3473:

Assert.Equal() Failure\r\n ↓ (pos 0)\r\nExpected: text\r\nActual: \r\n ↑ (pos 0)

Since clipboard tests are not [Collection("Sequential")] its possible another test (from a different class) cleared the clipboard and interfered with this test.

weltkante commented 4 years ago

Failed test TextBoxBase_Paste_InvokeNotEmpty_Success in CI run of PR #3511

RussKie commented 4 years ago

FYI I started looking into adding traits

JeremyKuhne commented 4 years ago

Failed again:

TextBoxBase_Paste_InvokeNotEmpty_Success
Assert.Equal() Failure\r\n           ↓ (pos 1)\r\nExpected: abc\r\nActual:   a\r\n           ↑ (pos 1)
 at System.Windows.Forms.Tests.TextBoxBaseTests.TextBoxBase_Paste_InvokeNotEmpty_Success() in /_/src/System.Windows.Forms/tests/UnitTests/TextBoxBaseTests.cs:line 5957
RussKie commented 2 years ago

One way of tracking rolling failures: https://runfo.azurewebsites.net/search/tests/?q=started%3A~14+definition%3A267+kind%3Arolling

RussKie commented 2 years ago

Related to https://github.com/dotnet/winforms/issues/6269

JeremyKuhne commented 1 year ago

This is a matter of making sure everything that might touch the clipboard is in a non-parallelized collection and ensuring that everything has sufficient retries. I've had some success in the clipboard tests in my thirtytwo project with them running stably. See: https://github.com/JeremyKuhne/thirtytwo/blob/main/src/thirtytwo_tests/ClipboardTestCollection.cs

Tanya-Solyanik commented 3 months ago

@LeafShi1 - you already fixed this in https://github.com/dotnet/winforms/pull/11606, right?

LeafShi1 commented 3 months ago

TextBoxBase_Paste_InvokeNotEmpty_Success

Yes