Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Improve snapcraft.yaml validation errors. #296
Conversation
elopio
reviewed
Feb 4, 2016
| @@ -283,7 +283,89 @@ def test_invalid_yaml_invalid_name_as_number(self, mock_loadPlugin): | ||
| snapcraft.yaml.Config() | ||
| self.assertEqual(raised.exception.message, | ||
| - '1 is not of type \'string\'') | ||
| + 'The \'name\' property does not match the required ' | ||
| + 'schema: 1 is not of type \'string\'') |
elopio
Feb 4, 2016
Member
If you use double quotes then you won't have to escape the single quotes. Same for the rest of the tests.
kyrofa
Feb 4, 2016
Member
Yeah, but jsonschema's error messages use single-quotes, so I had to bite the bullet and maintain consistency.
elopio
Feb 4, 2016
Member
right. I mean the exception to the rule of using single quotes to avoid me going nuts is when the string contains single quotes. In that case, use double quotes. Hah, confusing. So, the style we are trying to follow in the code is:
"The 'name' property does not match the required "
"schema: 1 is not of type 'string'"
Of course not all the old code is following it, as your deleted lines show.
|
Looks good, jsonschema could be better and make this easier instead of needing to go into hacks Let's wait for @elopio and the tests! |
|
@sergiusens heh, no kidding! I couldn't agree more. |
elopio
reviewed
Feb 4, 2016
| @jsonschema.FormatChecker.cls_checks('icon-path') | ||
| def _validate_icon(instance): | ||
| normalized_icon = instance.lower() | ||
| - ext = normalized_icon.endswith('.png') or normalized_icon.endswith('.svg') | ||
| - return os.path.exists(instance) and ext | ||
| + if not (normalized_icon.endswith('.png') or |
elopio
Feb 4, 2016
Member
You could use os.path.splitext once, and then just check for equality.
I'm not too convinced that it would be much clearer, so feel free to ignore.
sergiusens
Feb 4, 2016
Collaborator
if you do that, I'd prefer to use one of the fancy python things https://docs.python.org/3.5/library/functions.html#any
kyrofa
Feb 4, 2016
Member
Alright I revamped this a bit-- I think it's cleaner. Though @sergiusens I didn't see a good use for any here.
|
|
|
|
added a commit
that referenced
this pull request
Feb 4, 2016
kyrofa
merged commit dab07dd
into
snapcore:master
Feb 4, 2016
kyrofa
deleted the
kyrofa:bugfix/1524663/better_yaml_errors
branch
Feb 4, 2016
pushed a commit
to smoser/snapcraft
that referenced
this pull request
Sep 14, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
kyrofa commentedFeb 4, 2016
This PR fixes LP: #1524663 by improving YAML validation errors across the board. Specifically:
Was: "'icon.png' is not an icon-path"
Now: "Specified icon 'icon.png' does not exist" or "'icon' must be either a .png or a .svg"
Improve error message if apparmor or seccomp profiles don't exist.
Was: "The 'migration' property does not match the required schema: {'security-policy': {'apparmor': 'path/profile', 'seccomp': 'path/profile'}, 'type': 'migration-skill'} is not valid under any of the given schemas"
Now: "Specified file 'path/profile' does not exist"
Improve generic jsonschema errors to include the properties which had the error.
Was: "None is not of type 'object'"
Now: "The 'parts' property does not match the required schema: None is not of type 'object'"