-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Added regex test coverage #2640
Added regex test coverage #2640
Conversation
@mAAdhaTTah Could you please review this? I change a lot of files, so I kinda don't want to resolve merge conflicts for this. I would appreciate it if we could merge this quickly. The main reason so many language definitions change is that I found and fixed bugs. (As it turns out, untested code can contain bugs.) I also want to point out that the code coverage is by far not perfect. A regex (except for keyword lists) is considered covered if it has 1 match and since we don't know what parts of the pattern its matches cover, there is probably still a lot of untested code. I plan to address this problem in the future but the PR is already big enough for now. |
@RunDevelopment For the future, for big PRs like this, it would be helpful to ship it without whitespace or style changes. It would make it easier to review, as I wouldn't have to determine whether a change is style or functional. |
Sorry about that. |
JS File Size Changes (gzipped)A total of 19 files have changed, with a combined diff of +46 B (+0.2%).
|
It would be really nice if we could merge this. @mAAdhaTTah |
Closed in favor of #3138. |
This adds a little regex test coverage tool/test. It tracks all matches (= the result of
/some regex/.exec(code)
) of all of Prism's regexes on all language tests. If a regex doesn't produce any non-null matches, it will be reported. Reported regexes are grouped by language, so the error message will look like this:This also means that all languages without any tests will be reported (we had 3: GML, Ren'py, and Arduino).
The current coverage only reports completely untested patterns and simple keyword lists that are not exhaustively tested. We might want to expand this in the future. Those future applications are not part of this PR because it takes a lot of time to make all of Prism's >200 languages conform to a new standard.
Edit: The only reason I included a test for exhaustive keyword tests is that I only needed to add 13 tests. It was quite easy to add.
The main motivation for this PR is code review.
With this test, we will never merge a language with missing tests ever again.
I'm not quite done fixing all tests yet. I'll do the rest tomorrow or the day after.Edit: I'm done.