-
Notifications
You must be signed in to change notification settings - Fork 860
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
fix for stringify array replacer mozilla/rhino#725 #876
Conversation
Relevant tests which both pass are:
Tests are not yet run automatically, but are part of the effort in #817 |
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 seems fine to me, but I'd like to see that some tests are included. Does it make sense to wait and merge this after we get more tests from test262 working, or are there some other tests that you can add?
I can add a test using the examples from the linked issue. I didn't know if I needed to since it's going to be covered by test262, hopefully in the near-ish future. |
I plan to go forward with test262 updates in small steps on a weekly basis |
- Per spec, Numbers in an array replacer should be converted to strings prior to adding to the property list - Duplicates should be removed during construction of the property list - Changed some variables declared as List to Collection instead because that is sufficient for how they are being used - Moved List to array conversion from happening in every iteration of the jo method to being called once in the stringify method - Convert strings items representing integers to their Integer value for correct property lookup
052fbe1
to
2a3fd06
Compare
@gbrail I added some tests. The first time CircleCI tried to build, it failed with:
I reran the CircleCI build without changing any code, and it passed the second time. I'm guessing it was just a fluke with the rng. |
Works for me. Thanks! |
prior to adding to the property list
that is sufficient for how they are being used
the jo method to happening once in the stringify method
correct property lookup
fixes #725