Skip to content

Reliability improvement for input date E2E tests #35505

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

Merged
merged 5 commits into from
Aug 23, 2021

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Aug 19, 2021

I've run the Components E2E tests on a VM in a loop about 30 times, and the only ones that ever failed are:

  • InputDateInteractsWithEditContext_MonthInput
  • InputDateInteractsWithEditContext_NullableDateTimeOffset
  • InputDateInteractsWithEditContext_DateTimeLocalInput

They each only failed only 1 or 2 times over the 30 runs, but the fact that they are all roughly the same test suggests it's something problematic about those tests, and not just pure randomness. It's hard to know why they occasionally fail, but the screenshots captured in failing runs imply that their ApplyInvalidInputDateValue logic either doesn't run or has no effect when it does.

Strictly speaking, ApplyInvalidInputDateValue is a bit artificial because it's simulating doing something that an end user could not do. It's creating an invalid state (via JavaScript) that you couldn't actually create using mouse and keyboard gestures. So I'm going to make the case that it's not really a valid thing for us to do anyway.

In this PR,

  • I've removed ApplyInvalidInputDateValue from all the tests, and replaced it with real user gestures
  • In two cases, doing so caused the tests to fail consistently (which is a lot better than failing inconsistently!). I think this reflects an actual product defect so I've filed Keyboard input doesn't work for InputDateType.DateTimeLocal #35498 and am marking those tests as skipped here.

Now, with these improvements, I've run the full test suite in my VM another 10 times and it's had a 100% pass rate across that set. I'll keep the loop running.


Update

After running 78 more times, there were 4 failing runs:

  • 1 failed on ReloadingThePage_GracefullyDisconnects_TheCurrentCircuit
    • Fails with "collection was modified" enumeration error. Looking at the code, it's pretty obvious how a timing issue can make this fail. I've added a fix to this PR, and am fairly confident this is sorted.
  • 3 of which failed on CanFocusDuringOnAfterRenderAsyncWithFocusInEvent
    • It's not so clear how this can fail. My best guess is that there's some rare interference between renders and/or concurrent writes to the performFocusDuringOnAfterRender flag, since this test (very unusually) causes a render to be triggered from a background thread, and reads/writes that flag simultaneously on different threads.
    • I've added what is hopefully a fix in this PR, but will only know by running it a lot to see.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner August 19, 2021 15:58
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Aug 19, 2021
@SteveSandersonMS SteveSandersonMS added this to the 6.0-rc1 milestone Aug 19, 2021
@SteveSandersonMS SteveSandersonMS self-assigned this Aug 19, 2021
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/e2e-input-date-fixes branch from feec838 to e1bc416 Compare August 23, 2021 12:29
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/e2e-input-date-fixes branch from e1bc416 to 1591855 Compare August 23, 2021 12:55
@SteveSandersonMS SteveSandersonMS merged commit 610fb90 into main Aug 23, 2021
@SteveSandersonMS SteveSandersonMS deleted the stevesa/e2e-input-date-fixes branch August 23, 2021 17:05
@ghost ghost modified the milestones: 6.0-rc1, 7.0-preview1 Aug 23, 2021
@SteveSandersonMS
Copy link
Member Author

/backport to release/6.0-rc1

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/aspnetcore/actions/runs/1159547425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants