Mark remaining frequently-failing intermittent tests with FlakyTest attribute
#22246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peppy
reviewed
Jan 17, 2023
| @@ -73,6 +82,7 @@ public void TestNextItemSelectedAfterGameplayFinish() | |||
| } | |||
|
|
|||
| [Test] | |||
| [FlakyTest] // See above | |||
There was a problem hiding this comment.
Seems to have still failed even with the attribute..
|
I'm just going to get this in for now and we can figure out why the These flaky test failures should not be in our vision while we are working on other components so adding the attribute is a good first step even if it doesn't 100% work. |
peppy
approved these changes
Jan 17, 2023
|
Fyi I've also been taking intermittent looks into the multiplayer tests and have still not root caused it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resorting to the nuclear option here. I've spent some time investigating these remaining ones but to no avail, either due to me being literally burned-out from investigating anymore, or not experienced enough with the surrounding components to tell easily what's going on here.
I understand that some may not like going with this approach because it may be ignoring actual bugs at hand, but all of what I fixed doesn't show that to be the case (minus #22220), and I would rather use the attribute to enable myself merging PRs without re-running CI more than thrice in a row, than stare at my IDE not knowing of any idea on how to to fix the tests when it's nigh impossible to even reproduce it.
The fail rate is not included in the attached comments as it's no longer determinable with TeamCity's current state.
In addition, @ppy/area-client I propose we all follow some sort of guidelines to prevent this kind of catastrophe with CI from happening again in an effort to improve our workflow's efficiency. Something as simple as:
FlakyTestattributes to check whether any added test requires the attribute or otherwise (whether we could improve this process further is open for discussion).