Skip to content

Test cases for propertyNames with pattern. #452

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

Merged

Conversation

LeifRilbe
Copy link
Contributor

Added test cases for propertyNames with pattern.
Ref networknt/json-schema-validator#326

@notEthan
Copy link
Contributor

notEthan commented Jan 5, 2021

I think this is a good and valuable test to add. pattern is probably the most common keyword to be used
in a propertyNames schema, I would guess, so good to have in here.

I'd remove the tests for 'ignores string/array/etc' - I think these are covered by the tests both of the pattern keyword and the test above of propertyNames + maxLength.

one passinng and one failing test seems sufficient - the multiple fail cases feel more like testing the regex itself, rather than the application of the keyword. to that point, a less complex regex than the camelCase one might be simpler and more readable, such as the one used by the pattern tests.

note that this comment is not a change request. I'm not active enough in this repository to be telling anybody else what to write. just a couple of my thoughts.

@@ -43,6 +43,35 @@
}
]
},
{
"description": "propertyNames validation with pattern for camelCase",
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the reference to camel case in the test names since it seems we managed to simplify things?

After that yeah this looks great!

@Julian Julian merged commit 336ef8d into json-schema-org:master Jun 17, 2021
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