Skip to content
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

Test Additions for JS Array Functions #31

Closed

Conversation

SeunAdelekan
Copy link
Contributor

The Hermes VM currently has support for Javascript's Array.prototype.values() and Array.prototype.keys() functions but tests for them were yet to be included.

I worked on tests for both functions in this pull request.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 13, 2019
@dulinriley dulinriley requested a review from avp July 13, 2019 14:40
Copy link
Contributor

@avp avp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding tests is always helpful, thanks!

We already have some array iteration tests in
https://github.com/facebook/hermes/blob/master/test/hermes/iterator.js
though, so it would be preferred if you could move any tests for array iteration there instead.

@SeunAdelekan
Copy link
Contributor Author

Thanks @avp! I have added tests to the iterator.js file instead. Looking forward to your review.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avp is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@avp merged this pull request in 4f07c22.

facebook-github-bot pushed a commit that referenced this pull request Jan 14, 2020
Summary:
## Motivation

Why are you making this change?

What did you change?

How does the code work?

Why did you choose this approach?
Pull Request resolved: facebookincubator/fbjni#31

Test Plan:
How did you test this change?
Any change that adds functionality should add a unit test as well.

Reviewed By: BurntBrunch

Differential Revision: D19348810

Pulled By: passy

fbshipit-source-id: 7a144c80cc1078d968a562a17fa8d5f84047eeb2
@whatleo whatleo mentioned this pull request Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants