Skip to content

[Tests structure]: split tests for formats into multiple files #162

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

Closed
epoberezkin opened this issue Feb 12, 2017 · 6 comments · Fixed by #209
Closed

[Tests structure]: split tests for formats into multiple files #162

epoberezkin opened this issue Feb 12, 2017 · 6 comments · Fixed by #209

Comments

@epoberezkin
Copy link
Member

The rationale is that I could make multiple PRs for all additional formats and we can review/approve them separately. If I do multiple PRs into a single file they would likely conflict each other once some of them merged.

Given that going forward we will have some other additional formats it makes even more sense.

Also, they would be sorted alphabetically and we will see the list of formats.

@epoberezkin
Copy link
Member Author

@Julian what do you think?

If we do it, I am not sure what is a better way: have a separate folder for formats (that can probably break some tools, although only with draft-06) or have them named as format_uri.json for example. I'd probably have all the formats in optional/formats folder.

@awwright
Copy link
Member

There's not that many formats defined. We'd probably defining only one schema per test file if we broke it up, in which case what's the point?

@epoberezkin
Copy link
Member Author

There's not that many formats defined

@awwright I agree, but we are going to add several now (uri-reference, uri-template and json-pointer) and probably in the future too (date, time, etc.)

We're probably defining only one schema per test file

that's true but there are plenty of test cases so it becomes difficult to navigate the file

what's the point?

It's a bit of a hassle to make multiple PRs into the same file - there will be merge conflicts to resolve... Also it would allow seeing the list of formats for which we have tests without scrolling the file.

But I don't particularly insist on that, if you think it's a bad idea we can leave as is.

@Julian
Copy link
Member

Julian commented Feb 15, 2017

@epoberezkin no opinion -- I don't think it'll make too big of a difference one way or the other, and it's kind of nice that right now every validator corresponds to exactly 1 file, but if you do think it'll make your life easier, fine with me.

If you do do it, yeah, folder would be better.

@epoberezkin
Copy link
Member Author

Decided to leave it as is, closing :)

@epoberezkin
Copy link
Member Author

As draft-07 introduced many additional formats I am going to split format.json into multiple files to simplify testing/excluding unsupported formats. Currently ajv draft-07 tests exclude all formats.

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 a pull request may close this issue.

3 participants