-
-
Notifications
You must be signed in to change notification settings - Fork 219
fix optional test locations #382
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
Conversation
karenetheridge
commented
May 31, 2020
- move format tests to their own directory, for consistency
- move non-format tests out of the format directory that were put there by mistake
..except ECMA regex tests that don't use the "format" keyword
They were moved here by mistake in #365.
dd7b10a
to
e2248ce
Compare
These kinds of changes are backwards incompatible and intrusive, so we should be careful with them, but I've been aware of the need for this particular one since the folder structure was added to later drafts (truthfully it was probably best in retrospect to not have done that, but too late)... Will review this carefully but yeah we should be careful with this kind of change in general. |
This is mostly to fix a mistake I made a few weeks ago in #365 - not all the tests were "format"-related, doh! |
Sorry -- I clicked the button a bit too quick there -- I'd rather not split up the ecmascript-regex tests -- they all should live in one place I think. It's one unit of functionality (whether an implementation supports or doesn't support an ECMA regex engine). It's true something can support that but not support format, but that's exceedingly unlikely I think. So would rather we move the entirety of that file back where it was before, including the format-relevant pieces, so that it's all in once spot. Thoughts welcome though. |
I have no strong feelings either way. Do you want them all under format/, or outside of it? I'm not even sure if those ECMA 262 tests are accurate -- @ChALkeR had something to say about that recently. |
I'd prefer outside it I think, just by a bit (since that's how it was "before"). And yes merging this will conflict with @ChALkeR's PR but they'll forgive us :) we can fix the merge conflict if need be, before or after review. |
ok, done. I'll clean up the commits and rebase to master when it's approved. |
Handles the change done in json-schema-org/JSON-Schema-Test-Suite#382.