-
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
Convert HELPER_METHOD_FRAME in objectnative.cpp to QCall #97641
Changes from all commits
d108638
5c827a5
581bb9c
d3b9dcc
62fa673
4cc5699
6a56bd6
fb27520
8c45bd3
84ff6d1
46e9b7c
1bac399
e041418
e7bbdd4
2d46bc0
633f525
f2a0fc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -68,6 +68,26 @@ public static unsafe void GetObjectValue() | |||||||
Assert.Equal(i, (int)iOV); | ||||||||
} | ||||||||
|
||||||||
[Fact] | ||||||||
public static void EqualsTest() | ||||||||
{ | ||||||||
// Boolean RuntimeHelpers.Equals(Object, Object) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also cover comparing non-valuetypes and other paths that are special-cased in the implementation. |
||||||||
|
||||||||
Assert.True(RuntimeHelpers.Equals(Guid.Empty, Guid.Empty)); | ||||||||
Assert.False(RuntimeHelpers.Equals(Guid.Empty, Guid.NewGuid())); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
In case we'd generate an empty GUID. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||
|
||||||||
// Reference equal | ||||||||
object o = new object(); | ||||||||
Assert.True(RuntimeHelpers.Equals(o, o)); | ||||||||
|
||||||||
// Type mismatch | ||||||||
Assert.False(RuntimeHelpers.Equals(Guid.Empty, string.Empty)); | ||||||||
|
||||||||
// Non value types | ||||||||
Assert.False(RuntimeHelpers.Equals(new object(), new object())); | ||||||||
Assert.False(RuntimeHelpers.Equals(new int[] { 1, 2, 3 }, new int[] { 1, 2, 3 })); | ||||||||
} | ||||||||
|
||||||||
[Fact] | ||||||||
public static void InitializeArray() | ||||||||
{ | ||||||||
|
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 do not seem to have any test coverage for this method. Could you please add some?
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.
I'm not sure how to express the mentioned issue about padding bits.
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.
If we had any regular test coverage for this API, we would likely see the tests fail intermittently due to the padding issue.
I do not think you need to bother with adding a test that fails deterministically due the padding issue.