Add a few Marshal.*HGlobal tests#4162
Conversation
There was a problem hiding this comment.
Why is the class name of this test class called MarshalTests and yet the source filename doesn't match. The filename is called Marshal.cs ?
There was a problem hiding this comment.
I simply followed the naming scheme used by the rest of the project.
... or at least I thought I did. The rest of the project is inconsistent. I can update it.
There was a problem hiding this comment.
Thx. I always try to reconcile inconsistency when I see it if I'm adding/modifying more stuff to an existing project.
stephentoub
force-pushed
the
marshal_tests
branch
2 times, most recently
from
6629222 to
5fbbca5
Compare
There was a problem hiding this comment.
Shouldn't here be checked that a memory is readable/writeable?
There was a problem hiding this comment.
I do that in the test below; didn't seem like it would need to be done in every test. Here I just wanted to make sure that the various overloads of AllocHGlobal produced a non-zero IntPtr. I can update it, though.
There was a problem hiding this comment.
In next only writeable is checked. In L#45 you read from that is reallocated, so technically it's not the same. Maybe it is very strict, but if you hypothetically delete your reallocation test, then it Alloc method will be untested.
Implementing Marshal.ReAllocHGlobal in coreclr, and we didn't have any tests exercising it.
stephentoub
force-pushed
the
marshal_tests
branch
from
5fbbca5 to
c76364b
Compare
|
@dotnet-bot test this please (dependent functionality merged in coreclr) |
Add a few Marshal.*HGlobal tests
Add a few Marshal.*HGlobal tests Commit migrated from dotnet/corefx@7086e5c
5 participants
I'm implementing Marshal.ReAllocHGlobal in coreclr on Unix, and we didn't have any tests exercising it.
(This will fail the Linux CI leg until the change has been PR'd through coreclr.)