Add ConditionalWeakTable.AddOrUpdate to contract and add tests#14661
Conversation
safern
added
api-approved
|
Thanks for reviewing @jkotas :) |
karelz
removed
the
api-approved
|
It fixes #8429 (which was api-approved). |
|
Sounds good :) @karelz |
| Assert.True(cwt.TryGetValue(key, out value)); | ||
| Assert.Equal(value, "value1"); | ||
| Assert.Equal(value, cwt.GetOrCreateValue(key)); | ||
| Assert.Equal(value, cwt.GetValue(key, k => "value1")); |
There was a problem hiding this comment.
This assert isn't necessarily testing what you want it to test. GetValue will look up key and return its value if it exists; if it doesn't exist, it'll call the specified lambda to create the value for the key, add it to the table, and then return that added value. But the value returned from the lambda is the same as the one previously stored and expected for the key; if there was a bug that caused GetValue to invoke the lambda even if the key was in the table, this assert would still pass. The lambda should really return some other value.
5 participants
Fixes #8429
Add test and update contract for pull dotnet/coreclr#8490
/cc @jkotas @stephentoub