-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix the flakiness of new BinaryFormatter tests #103476
Conversation
…ont as it enumerates all properties and may change the contents of the fields
…ches to be the same object, as we want to compare exactly the same object on purpose
…uated SortedList<TKey, TValue>.keyList and valueList
…e it which may cause hard to repro failures based on the order of executed tests
Tagging subscribers to this area: @dotnet/area-system-resources |
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.
We just got "lucky" that we didn't hit this before in existing tests?
These tests were not caching the input for all tests in a static field, so the mutation could not affect other test invocations |
Recently I've noticed that the new BF tests fail from time to time, claiming that the blobs stored in the tests became outdated.
Sample log entry:
It was very hard to reproduce this problem, so my guess was that it was dependent on test execution order.
I did some debugging and it turned out that
EqualityExtensions.CheckEquals
enumerates over all properties of given type.Some of the properties might be lazy evaluated and stored in a field. BF is storing all the fields. It means that serializing given object instance before and after passing it to
EqualityExtensions.CheckEquals
may produce different blobs!The fix was to call that method up-front and update the blob and moreover stop caching the test input, as it may clearly get mutated by the tests (this explains why the original BF tests were not flaky).
fixes #103367
fixes #103478