Skip to content

Commit

Permalink
pr feedback, reduce error message to a single line
Browse files Browse the repository at this point in the history
  • Loading branch information
bepri committed Dec 4, 2024
1 parent 20b0ae5 commit dadb59a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 50 deletions.
24 changes: 5 additions & 19 deletions snapcraft/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import copy
import re
import textwrap
from typing import Any, Literal, Mapping, Tuple, cast

import pydantic
Expand Down Expand Up @@ -441,29 +440,16 @@ def _validate_autostart_name(cls, name):
"command", "stop_command", "post_stop_command", "reload_command", "bus_name"
)
@classmethod
def _validate_apps_section_content(cls, command: str, info: pydantic.ValidationInfo) -> str:
def _validate_apps_section_content(
cls, command: str, info: pydantic.ValidationInfo
) -> str:
# Find any invalid characters in the command.
# The regex below is derived from snapd's validator code, modified to be the inverse (^).
# https://github.com/canonical/snapd/blob/0706e2d0b20ae2bf030863f142b8491b66e80bcb/snap/validate.go#L756
bad_chars: set[str] = set()
for bad_char in re.finditer(r"[^A-Za-z0-9/. _#:$-]", command):
bad_chars.add(bad_char.group(0))

if bad_chars:
if not re.match(r"^[A-Za-z0-9/. _#:$-]$", command):
# Guaranteed not-none as the pydantic field_validator decorator always populates it.
deserialized = cast(str, info.field_name).replace("_", "-")
bad_chars_print = ", ".join(sorted(bad_chars))
doc_link = (
"https://snapcraft.io/docs/snapcraft-yaml-schema#p-21225-appsapp-name"
)
message = textwrap.dedent(
f"""\
{deserialized}: App commands must consist only of alphanumeric characters, spaces, and the following characters:
/ . _ # : $ -
The following characters were encountered, but not allowed: {bad_chars_print}
For more complete details about the `command` key, see: {doc_link}
"""
)
message = f"{deserialized}: App commands must consist of only alphanumeric characters, spaces, and the following characters: / . _ # : $ -"
raise ValueError(message)

return command
Expand Down
40 changes: 9 additions & 31 deletions tests/unit/models/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import itertools
import textwrap
from typing import Any, Dict, cast

import pydantic
Expand Down Expand Up @@ -1439,18 +1438,10 @@ def test_project_provenance_invalid(self, provenance, project_yaml_data):
"bus_name",
],
)
@pytest.mark.parametrize(
"value",
[
pytest.param(
"mkbird --chirps 5",
None,
id="good",
),
]
)
def test_app_command_lexicon_good(
self, app_yaml_data, key: str, value: str
self,
app_yaml_data,
key: str,
):
"""Verify that command validation lets in a valid command."""
command = {key: "mkbird --chirps 5"}
Expand All @@ -1459,7 +1450,7 @@ def test_app_command_lexicon_good(

# Ensure the happy path
assert proj.apps is not None
assert getattr(proj.apps["app1"], key) == value
assert getattr(proj.apps["app1"], key) == "mkbird --chirps 5"

@pytest.mark.parametrize(
"key",
Expand All @@ -1472,39 +1463,26 @@ def test_app_command_lexicon_good(
],
)
@pytest.mark.parametrize(
"value,bad_chars",
"value",
[
pytest.param(
"mkbird --chirps=5",
["="],
id="has_bad_char",
),
pytest.param(
'mkbird --chirps=1337 --name="81U3J@Y"', ['"', "@", "="], id="many_bad"
),
pytest.param('mkbird --chirps=1337 --name="81U3J@Y"', id="many_bad"),
],
)
def test_app_command_lexicon_bad(self, app_yaml_data, key: str, value: str, bad_chars: list[str]):
def test_app_command_lexicon_bad(self, app_yaml_data, key: str, value: str):
"""Verify that invalid characters in command fields raise an error."""
command = {key: value}
data = app_yaml_data(**command)
doc_link = (
"https://snapcraft.io/docs/snapcraft-yaml-schema#p-21225-appsapp-name"
)

err_msg = textwrap.dedent(
f"""\
{key.replace('_', '-')}: App commands must consist only of alphanumeric characters, spaces, and the following characters:
/ . _ # : $ -
The following characters were encountered, but not allowed: {', '.join(sorted(bad_chars))}
For more complete details about the `command` key, see: {doc_link}
"""
)
err_msg = f"{key.replace('_', '-')}: App commands must consist of only alphanumeric characters, spaces, and the following characters: / . _ # : $ -"

with pytest.raises(pydantic.ValidationError) as val_err:
Project.unmarshal(data)

assert err_msg in str(val_err.value)
assert err_msg in str(val_err.value)


class TestGrammarValidation:
Expand Down

0 comments on commit dadb59a

Please sign in to comment.