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

Add staging source-phase-imports #4244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

legendecas
Copy link
Member

Even though https://github.com/tc39/proposal-esm-phase-imports will supersede this test case, as https://tc39.es/proposal-source-phase-imports/#sec-source-text-module-record-getmodulesource is defined in the spec, add a staging test to cover import a SourceTextModule source.

Refs: #3980 (review)

@legendecas legendecas requested review from a team as code owners October 2, 2024 23:11
@syg
Copy link
Contributor

syg commented Oct 2, 2024

Wait, you don't need to manually add staging tests if you're working in V8. You can add these as part of the V8 CL and there's a 2-way sync bot that'll take care of the exporting. In that case you can run these tests as part of the V8 CQ.

@legendecas
Copy link
Member Author

legendecas commented Oct 2, 2024

Yeah, I tried that. But the tools (and v8 test runner) complain about the folders that do not exist in test262 repo. So I manually picked the change here.

@ptomato
Copy link
Contributor

ptomato commented Oct 2, 2024

There's no link to the V8 CL but if someone reviewed these there, I'm happy to click the merge button if for whatever reason the bot can't do it.

@syg
Copy link
Contributor

syg commented Oct 2, 2024

There's no link to the V8 CL but if someone reviewed these there, I'm happy to click the merge button if for whatever reason the bot can't do it.

Please hold off for a little bit. I want to make sure whether there's a bug / unsupported use case with the 2-way sync bot.

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.

3 participants