-
Notifications
You must be signed in to change notification settings - Fork 971
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
Removed extra validation in flaky DataGridView unit tests. #8574
Removed extra validation in flaky DataGridView unit tests. #8574
Conversation
ea1f263
to
65cfd58
Compare
@dreddy-work - how do you feel about having commented out code in tests? I'm against it because the issue is already documenting the problem. I would remove the unused code and data and keep the reference to the issue. |
65cfd58
to
6cbbccd
Compare
Given @merriemcgaw opinion here I've decided we'd better get rid of commented out code here (and change tests theories subsequently). |
I have nothing against it, but such an approach means that these tests will never be returned :) |
6cbbccd
to
ea4765d
Compare
But we will still have this issue and all changes recorded, aren't we? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Perhaps we should keep the issue #7799 open until we fix the underlying issue with these tests and then bring back removed checks?
Makes sense. Please mark the issue appropriately, or better - open a new one dedicated to the |
Already here. 🫡 This PR not fix it (opposite to described in 1 post). |
Fixes #7799 (the main Issue containing a complete list of flaky tests)
Fixes #6597
Fixes #6739
Fixes #6926 (along with #8535)
Proposed changes
invalidatedCallCount
in unstable unit tests with a commentary reference to this Issue to keep a track record (as proposed by @RussKie).Tests changed:
Customer Impact
Regression?
Risk
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow