The Wayback Machine - https://web.archive.org/web/20201003210816/https://github.com/facebook/hermes/pull/31
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

@SeunAdelekan SeunAdelekan commented Jul 13, 2019

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.

@dulinriley dulinriley requested a review from avp Jul 13, 2019
Copy link
Contributor

@avp avp left a comment

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

@SeunAdelekan SeunAdelekan commented Jul 14, 2019

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

@avp
avp approved these changes Jul 15, 2019
Copy link
Contributor

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

@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

@facebook-github-bot facebook-github-bot commented Jul 15, 2019

@avp merged this pull request in 4f07c22.

facebook-github-bot added 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.