Skip to content
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

Disable parallelism on BootResourceCachingTest #30395

Merged

Conversation

SteveSandersonMS
Copy link
Member

Tests in BootResourceCachingTest have been quarantined for a long time due to flakiness. Even after several attempts to fix this (#20154 and #27374), they continue to fail about 1 in 10 times. Somehow, the browser's cache occasionally doesn't include files we expect to have been cached, or it does include a file that should not yet be cached (suggesting that the test browser isn't as isolated as it should be). Maybe the browser doesn't always commit the cache updates to disk in a synchronous and blocking manner, so sometimes the expected cache updates haven't completed by the time we try to observe them.

The nuclear solutions might include:

  • Put in a retry mechanism, and agree it's good enough if the test passes either first time or N-out-of-M times
  • Or, rewrite the test using PlayWright which gives us more direct visibility into the HTTP traffic (but still doesn't guarantee anything about how the browser's caches get committed)

As one final attempt to avert those, this PR disables parallelism for those tests. It's quite possible that if the test environment isn't under heavy load at the time it runs these tests, then the browser's cache storage might behave predictably. We can see if this is reliable enough to justify unquarantining the tests.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Feb 23, 2021
@SteveSandersonMS SteveSandersonMS added this to the 6.0-preview2 milestone Feb 23, 2021
@SteveSandersonMS SteveSandersonMS requested a review from a team February 23, 2021 12:06
@SteveSandersonMS SteveSandersonMS self-assigned this Feb 23, 2021
@@ -18,6 +18,9 @@

namespace Microsoft.AspNetCore.Components.E2ETest.Tests
{
// Disabling parallelism for these tests because of flakiness
[CollectionDefinition(nameof(BootResourceCachingTest), DisableParallelization = true)]
[Collection(nameof(BootResourceCachingTest))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that this collection runs sequentially from any other collection and tests that are not in a collection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is my understanding based on the Xunit docs.

@javiercn
Copy link
Member

  • Or, rewrite the test using PlayWright which gives us more direct visibility into the HTTP traffic (but still doesn't guarantee anything about how the browser's caches get committed)

That might be an option, although I'm not sure if the request will be emitted in that case anyways even if it's served from the cache.

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one question, but lets give this a try

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