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

Fix phase on some negative module tests #3832

Merged
merged 2 commits into from
Jun 10, 2023

Conversation

jedel1043
Copy link
Contributor

@jedel1043 jedel1043 commented May 22, 2023

A couple of negative tests had an incorrect phase tagged, since their errors happen at linking time, which is part of the "resolution" phase. However, they were marked as happening at runtime.

This PR fixes those tests and additionally adds a $DONOTEVALUATE marker to them.

Copy link
Contributor

@jugglinmike jugglinmike left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

Out of curiosity: do you have an environment that was failing the erroneous version of these tests? Or did you spot the mistake through careful reading?

@jugglinmike jugglinmike enabled auto-merge (squash) June 10, 2023 22:09
@jugglinmike jugglinmike enabled auto-merge (squash) June 10, 2023 22:10
@jugglinmike jugglinmike merged commit c4642dd into tc39:main Jun 10, 2023
@jedel1043
Copy link
Contributor Author

jedel1043 commented Jun 10, 2023

Or did you spot the mistake through careful reading?

I wish 😅

Out of curiosity: do you have an environment that was failing the erroneous version of these tests?

I'm part of the team developing Boa, an experimental JS engine written in Rust. We found out about the bugs when we were finishing our implementation of the ES modules spec
(boa-dev/boa#2955)

Internally, we have a testing tool that uses test262 to know our current conformance at any point while developing.

@jedel1043 jedel1043 deleted the fix-resolution-phase branch June 10, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants