-
Notifications
You must be signed in to change notification settings - Fork 688
Seperate tests from test-api.c - Strings #2564
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
Conversation
LaszloLango
left a comment
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
zherczeg
left a comment
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.
Good, only a few changes needed.
tests/unit-core/test-api-strings.c
Outdated
| int | ||
| main (void) | ||
| { | ||
|
|
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.
Remove this newline.
tests/unit-core/test-api-strings.c
Outdated
|
|
||
| static bool | ||
| strict_equals (jerry_value_t a, | ||
| jerry_value_t b) |
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.
Add description to arguments (I know it is copied, but now we have a stricter style rules).
tests/unit-core/test-api-strings.c
Outdated
| * limitations under the License. | ||
| */ | ||
|
|
||
| #include "config.h" |
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.
Is this picking up jerry-core/config.h? As this is an API test and config.h is not part of the public API, it should not be used (or needed).
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.
The include came from the original test-api.c file, as i initially moved code around, and looks like i forgot to delete it, my bad, will fix it in a moment.
`test-api.c` is containing the testing for all API functions, which resulted in a big and hardly readable file. This PR is the first of many, that tries to segment it into more files, by tested functionality, this time strings and string methods. JerryScript-DCO-1.0-Signed-off-by: Bela Toth tbela@inf.u-szeged.hu
akosthekiss
left a comment
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
zherczeg
left a comment
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
test-api.cis containing the testing for all API functions, which resulted in a big and hardly readable file.This PR is the first of many, that tries to segment it into more files, by tested functionality, this time strings and string methods.
JerryScript-DCO-1.0-Signed-off-by: Bela Toth tbela@inf.u-szeged.hu