-
Notifications
You must be signed in to change notification settings - Fork 28
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
#993. [FFI] Missed tests added for some new types #1384
Conversation
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 with comments
LibTest/ffi/Bool/Bool_A01_t01.dart
Outdated
void main() { | ||
Pointer<Bool> p1 = calloc<Bool>(); | ||
try { | ||
Expect.isFalse(p1.value); |
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.
This tests calloc
behavior. If you intended to only test Bool
, remove this. (malloc
doesn't zero out the memory. calloc
zeroes out the memory, which happens to be false.)
LibTest/ffi/Bool/Bool_A01_t02.dart
Outdated
|
||
void main() { | ||
Pointer<Bool> p1 = calloc<Bool>(); | ||
Pointer<Int16> p2 = new Pointer<Int16>.fromAddress(p1.address); |
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.
All the tests below rely on a little endian encoding of the number: the least significant byte is used if only one byte is read.
This happens to succeed right now because we do not support any big endian architectures in Dart at the moment.
Instead, use Uint8 to avoid big/little endian issues in this test.
void main() { | ||
Pointer<Bool> p1 = calloc<Bool>(3); | ||
try { | ||
Expect.equals(1, p1.elementAt(1).address - p1.address); |
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.
Maybe add sizeOf<Bool>()
here as well, because that's what you're testing.
LibTest/ffi/Long/Long_A01_t01.dart
Outdated
Expect.equals(42, p1.value); | ||
p1.value = 32768; | ||
Expect.equals(32768, p1.value); | ||
p1.value = 2147483647; // 0x7FFFFFFF |
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.
Dart supports doing 0x7FFFFFFF instead, that might make the test more readable.
void main() { | ||
Pointer<Long> p1 = calloc<Long>(2); | ||
try { | ||
Expect.isTrue(p1.elementAt(1).address - p1.address == 4 || |
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.
sizeOf
void main() { | ||
Pointer<LongLong> p1 = calloc<LongLong>(3); | ||
try { | ||
Expect.equals(8, p1.elementAt(1).address - p1.address); |
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.
sizeOf()
Thank you. I did the changes according all of the notes |
It is not about whether the types are signed, but whether they are larger than the type being tested.
Thanks @sgrekhov lgtm! |
2022-08-04 sgrekhov22@gmail.com dart-lang/co19#993. FFI tests refactored and improved (dart-lang/co19#1387) 2022-08-04 sgrekhov22@gmail.com dart-lang/co19#993. [FFI] Missed tests added for some new types (dart-lang/co19#1386) 2022-08-03 sgrekhov22@gmail.com Fixes dart-lang/co19#1129. Don't expect any proxy routine in case of DIRECT connection (dart-lang/co19#1383) 2022-08-03 sgrekhov22@gmail.com dart-lang/co19#993. [FFI] Missed tests added for some new types (dart-lang/co19#1384) 2022-07-29 sgrekhov22@gmail.com dart-lang/co19#673. IFrame tests that don't actually test IFrame removed (dart-lang/co19#1382) 2022-07-29 sgrekhov22@gmail.com dart-lang/co19#1363. Some delays removed or reduced (dart-lang/co19#1380) 2022-07-29 sgrekhov22@gmail.com dart-lang/co19#1363 Async `for-in` test updated according to the current specification (dart-lang/co19#1379) 2022-07-27 sgrekhov22@gmail.com dart-lang/co19#1363. Yield each tests updated to the current specification (dart-lang/co19#1374) 2022-07-27 sgrekhov22@gmail.com Fixes dart-lang/co19#1375. Add test for final variable in a for-in loop (dart-lang/co19#1378) 2022-07-25 sgrekhov22@gmail.com dart-lang/co19#1363. Tests for yield updated to the current specification (dart-lang/co19#1370) Change-Id: Ie92f4b2157284777177012320883247411e214fa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254140 Reviewed-by: Erik Ernst <eernst@google.com> Commit-Queue: Alexander Thomas <athom@google.com>
No description provided.