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

Make IncrementallyUpdatesCache and CachesResourcesAfterFirstLoad tests more robust. Hopefully fixes #20154 #29091

Merged

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Jan 6, 2021

I don't know for certain why these tests occasionally fail, because they pass ~98% of the time in CI, and 100% of the times I've tried running locally.

However I notice that the collection in which we track the requested URLs is not thread-safe, and is potentially written from multiple threads concurrently. This could explain why sometimes we don't find the expected URLs in the collection.

I've also added a second tweak to make sure that, when we navigate to about:blank, we wait until that navigation is completed before proceeding to re-navigate back to where we came from.

Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

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

Do you need to unquarantine the test?

@SteveSandersonMS
Copy link
Member Author

@pranavkm I thought the process was to let the unquarantining system do that once the change has been proven effective over time. Have we changed that process?

@HaoK
Copy link
Member

HaoK commented Jan 6, 2021

Yeah I think we are supposed to wait for 30 days of clean running before unquarantining, so if this test at 98% recently, need to remember to check back and unquarantine later, since I'm not sure how automatic the unquarantining process is, so it seems better to follow up explicitly.

@SteveSandersonMS SteveSandersonMS merged commit 062ef7d into master Jan 7, 2021
@SteveSandersonMS SteveSandersonMS deleted the stevesa/20154-flaky-test-IncrementallyUpdatesCache branch January 7, 2021 10:44
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.

3 participants