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

Update RegExp.prototype.{match,replace} tests to expect Get(rx, "flags") #3618

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

gibson042
Copy link
Contributor

@gibson042 gibson042 commented Jul 27, 2022

This adds tests for tc39/ecma262#2791 , which reached consensus at the July 2022 TC39 meeting.

@gibson042
Copy link
Contributor Author

This results in failures, as expected:

results
FAIL test/built-ins/RegExp/prototype/Symbol.match/flags-tostring-error.js (default)
  Expected a CustomError but got a Test262Error

FAIL test/built-ins/RegExp/prototype/Symbol.match/get-flags-err.js (default)
  Expected a CustomError but got a Test262Error

FAIL test/built-ins/RegExp/prototype/Symbol.match/get-unicode-error.js (default)
  Expected a Test262Error to be thrown but no exception was thrown at all

FAIL test/built-ins/RegExp/prototype/Symbol.replace/flags-tostring-error.js (default)
  Expected a CustomError but got a Test262Error

FAIL test/built-ins/RegExp/prototype/Symbol.replace/get-flags-err.js (default)
  Expected a CustomError but got a Test262Error

FAIL test/built-ins/RegExp/prototype/Symbol.replace/get-unicode-error.js (default)
  Expected a Test262Error to be thrown but no exception was thrown at all

Do we have instructions for how to test such changes by modifying engine262?

@ptomato
Copy link
Contributor

ptomato commented Jul 27, 2022

We don't have those instructions yet. That would be a great tutorial to have!

mathiasbynens
mathiasbynens previously approved these changes Jul 29, 2022
@gibson042
Copy link
Contributor Author

@ptomato Do you have such experience? I don't, but I'd be happy to write something up from either a past example or a new pairing session.

@ptomato
Copy link
Contributor

ptomato commented Aug 1, 2022

I don't either, and would love to learn. IIRC @devsnek once gave a demonstration of implementing a small proposal in engine262, so maybe they could help?

@devsnek
Copy link
Member

devsnek commented Aug 1, 2022

@gibson042 maybe drop by https://matrix.to/#/#engine262:matrix.org and we can discuss? this should hopefully be quite straight-forward but i'd love to hear your feedback on the process.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM

@gibson042
Copy link
Contributor Author

With help from @devsnek, I was able to verify that engine262 modifications to implement tc39/ecma262#2791 result in it passing all tests broken by this PR. 🎉

@ljharb ljharb force-pushed the ecma262-2791-regexp-method-flags branch from 6296208 to 4ddf9d1 Compare August 4, 2022 20:55
@ljharb ljharb requested a review from a team as a code owner August 4, 2022 20:55
@ptomato ptomato force-pushed the ecma262-2791-regexp-method-flags branch from 4ddf9d1 to 4ae9a9e Compare August 9, 2022 18:57
@ptomato ptomato merged commit 9e51a9d into tc39:main Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants