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

Import and modernize bocoup/test262-regexp-generator #4303

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Nov 2, 2024

https://github.com/bocoup/test262-regexp-generator was used to generate the tests in test/built-ins/RegExp/CharacterClassEscapes/. It makes sense for this script to live in the same repo as the tests it generates. Especially since in the meantime, the tests themselves have gotten out of sync with the upstream.

This imports the script, modernizes and cleans up the code, implements the changes necessary to match the changes made in the meantime to the tests, and integrates the script into the build process.

Instead of an output/ folder, output the tests directly into the folder
where they live.
Add some minimal instructions for regenerating the tests.
This code hasn't been touched in a while, so it's probably good to bring
in the newest versions of the dependencies. We can easily tell if there
was any incompatible effect on the output.

The latest version of filenamify requires using ES modules. We also have
to adapt to a breaking change in regexpu-core (see
mathiasbynens/regexpu-core#49).

Also convert the dependencies to devDependencies, since this tool is not
necessary for executing test262.
@ptomato
Copy link
Contributor Author

ptomato commented Nov 2, 2024

Unfortunately, ci_build.sh fails due to my changes to make.py but that does not result in the CI job failing. I'll sort that out later.

@ptomato ptomato force-pushed the bocoup-test262-regexp-generator branch from 90388c6 to 3b31f3c Compare November 2, 2024 01:32
Copy link
Member

@mathiasbynens mathiasbynens 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 your work on this!

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

lgtm modulo other comments

tools/regexp-generator/index.mjs Outdated Show resolved Hide resolved
The optimizations from commit e558b29 were never incorporated into the
upstream test generator. This does so now.

As far as I can tell, the changes to the Unicode ranges are purely
cosmetic. Some are formatted as 6-digit hex numbers instead of 4-digit.
Others move the low-surrogates range 0xDC00-0xDCFF to the beginning of the
array, but the union of the ranges is still the same.
These are unused parameters and variables, and all have no effect on the
output.
Add the `generated` flag, and update the link to the generator script.
This is necessary so that updates to the tests without corresponding
updates to the generator script will be flagged in CI runs.
As per the package.json docs, these are optional if the package is not
being published.
@ptomato ptomato force-pushed the bocoup-test262-regexp-generator branch from 90388c6 to bef3fb3 Compare November 4, 2024 18:48
While we're touching this we may as well update the quote from ECMA-262 to
match what it currently says.
@ptomato ptomato force-pushed the bocoup-test262-regexp-generator branch 3 times, most recently from f820b08 to 37c1b86 Compare November 4, 2024 20:19
Our config files specify two-space indents for JS files. These scripts
were probably written before that was a thing. Update the indentation of
the script and the generated tests all in one go.
Applying only to .travis.yml seems to be a relic from when that was the
only YAML file in the repo. Extend it to all YAML files.

Likewise, we have .mjs files now in the regexp-generator tool. Extend the
same indentation from .js files to cover them as well.
@ptomato ptomato force-pushed the bocoup-test262-regexp-generator branch 3 times, most recently from e34e9e0 to 19da29a Compare November 4, 2024 21:54
@ptomato ptomato force-pushed the bocoup-test262-regexp-generator branch from 19da29a to 31ddf4f Compare November 4, 2024 21:58
@ptomato
Copy link
Contributor Author

ptomato commented Nov 4, 2024

Rather than mess with the Circle-CI images to get one that has both Python and Node installed, I'll migrate the linter and test generation jobs to GitHub Actions where they can run on the ubuntu-latest image which has everything already. I'll do that in a separate PR, and hopefully make some improvements along the way. Let's consider this PR on hold for the moment.

(Context: the make.py failure was due to the Circle-CI image not having Node installed)

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.

4 participants