Skip to content
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

Adjust add-on input validation #3263

Merged
merged 22 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 46 additions & 20 deletions kedro/framework/cli/starters.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,7 @@ def _validate_range(start, end):
click.secho(message, fg="red", err=True)
sys.exit(1)

def _validate_selection(add_ons: list[str]):
for add_on in add_ons:
if add_on not in NUMBER_TO_ADD_ONS_NAME:
message = f"'{add_on}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7." # nosec
click.secho(message, fg="red", err=True)
sys.exit(1)

add_ons_str = add_ons_str.lower()
if add_ons_str == "all":
return list(NUMBER_TO_ADD_ONS_NAME)
if add_ons_str == "none":
Expand All @@ -240,7 +234,7 @@ def _validate_selection(add_ons: list[str]):
return [] # pragma: no cover

# Split by comma
add_ons_choices = add_ons_str.split(",")
add_ons_choices = add_ons_str.replace(" ", "").split(",")
selected: list[str] = []

for choice in add_ons_choices:
Expand All @@ -251,7 +245,6 @@ def _validate_selection(add_ons: list[str]):
else:
selected.append(choice.strip())

_validate_selection(selected)
return selected


Expand Down Expand Up @@ -316,6 +309,10 @@ def new( # noqa: PLR0913
cookiecutter_dir = _get_cookiecutter_dir(template_path, checkout, directory, tmpdir)
prompts_required = _get_prompts_required(cookiecutter_dir)

# Format user input where necessary
if selected_addons is not None:
selected_addons = selected_addons.lower()

# Select which prompts will be displayed to the user based on which flags were selected.
prompts_required = _select_prompts_to_display(
prompts_required, selected_addons, project_name
Expand Down Expand Up @@ -447,7 +444,7 @@ def _convert_addon_names_to_numbers(selected_addons: str) -> str:
return None

addons = []
for addon in selected_addons.split(","):
for addon in selected_addons.lower().split(","):
addon_short_name = addon.strip()
if addon_short_name in ADD_ONS_SHORTNAME_TO_NUMBER:
addons.append(ADD_ONS_SHORTNAME_TO_NUMBER[addon_short_name])
Expand Down Expand Up @@ -762,6 +759,16 @@ def _make_cookiecutter_context_for_prompts(cookiecutter_dir: Path):
return cookiecutter_context.get("cookiecutter", {})


def _validate_selection(add_ons: list[str]):
# start validating from the end, when user select 1-20, it will generate a message
# '20' is not a valid selection instead of '8'
for add_on in add_ons[::-1]:
if add_on not in NUMBER_TO_ADD_ONS_NAME:
message = f"'{add_on}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7." # nosec
click.secho(message, fg="red", err=True)
sys.exit(1)


class _Prompt:
"""Represent a single CLI prompt for `kedro new`"""

Expand All @@ -786,12 +793,17 @@ def __str__(self) -> str:

def validate(self, user_input: str) -> None:
"""Validate a given prompt value against the regex validator"""
if self.regexp and not re.match(self.regexp, user_input):

if self.regexp and not re.match(self.regexp, user_input.lower()):
message = f"'{user_input}' is an invalid value for {(self.title).lower()}."
click.secho(message, fg="red", err=True)
click.secho(self.error_message, fg="red", err=True)
sys.exit(1)

if self.title == "Project Add-Ons":
# Validate user input
_validate_selection(_parse_add_ons_input(user_input))


def _get_available_tags(template_path: str) -> list:
# Not at top level so that kedro CLI works without a working git executable.
Expand Down Expand Up @@ -841,24 +853,38 @@ def _validate_config_file_against_prompts(


def _validate_config_file_inputs(config: dict[str, str]):
"""Checks that variables provided through the config file are of the expected format.
"""Checks that variables provided through the config file are of the expected format. This
validate the config provided by `kedro new --config` in a similar way to `prompts.yml`
for starters.

Args:
config: The config as a dictionary.
config: The config as a dictionary

Raises:
SystemExit: If the provided variables are not properly formatted.
"""
project_name_reg_ex = r"^[\w -]{2,}$"
project_name_validation_config = {
"regex_validator": r"^[\w -]{2,}$",
"error_message": "'{input_project_name}' is an invalid value for project name. It must contain only alphanumeric symbols, spaces, underscores and hyphens and be at least 2 characters long",
}

input_project_name = config.get("project_name", "New Kedro Project")
if not re.match(project_name_reg_ex, input_project_name):
message = f"'{input_project_name}' is an invalid value for project name. It must contain only alphanumeric symbols, spaces, underscores and hyphens and be at least 2 characters long"
click.secho(message, fg="red", err=True)
if not re.match(
project_name_validation_config["regex_validator"], input_project_name
):
click.secho(project_name_validation_config["error_message"], fg="red", err=True)
sys.exit(1)

add_on_reg_ex = r"^(all|none|(\d(,\d)*|(\d-\d)))$"
input_add_ons = config.get("add_ons", "none")
if not re.match(add_on_reg_ex, input_add_ons):
message = f"'{input_add_ons}' is an invalid value for project add-ons. Please select valid options for add-ons using comma-separated values, ranges, or 'all/none'."
add_on_validation_config = {
"regex_validator": r"^(all|none|(( )*\d*(,\d*)*(,( )*\d*)*( )*|( )*((\d+-\d+)|(\d+ - \d+))( )*))$",
"error_message": f"'{input_add_ons}' is an invalid value for project add-ons. Please select valid options for add-ons using comma-separated values, ranges, or 'all/none'.",
}

if not re.match(add_on_validation_config["regex_validator"], input_add_ons.lower()):
message = add_on_validation_config["error_message"]
click.secho(message, fg="red", err=True)
sys.exit(1)

selected_add_ons = _parse_add_ons_input(input_add_ons)
_validate_selection(selected_add_ons)
2 changes: 1 addition & 1 deletion kedro/templates/project/prompts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ add_ons:
7) Kedro Viz: Provides Kedro's native visualisation tool

Which add-ons would you like to include in your project? [1-7/1,3/all/none]:
regex_validator: "^(all|none|(\\d(,\\d)*|(\\d-\\d)))$"
regex_validator: "^(all|none|(( )*\\d*(,\\d*)*(,( )*\\d*)*( )*|( )*((\\d+-\\d+)|(\\d+ - \\d+))( )*))$"
error_message: |
Invalid input. Please select valid options for add-ons using comma-separated values, ranges, or 'all/none'.

Expand Down
144 changes: 129 additions & 15 deletions tests/framework/cli/test_starters.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
KedroStarterSpec,
_convert_addon_names_to_numbers,
_parse_add_ons_input,
_validate_selection,
)

FILES_IN_TEMPLATE_WITH_NO_ADD_ONS = 15
Expand Down Expand Up @@ -81,7 +82,6 @@ def _get_expected_files(add_ons: str):
"7": 0, # Kedro Viz does not add any files
} # files added to template by each add-on
add_ons_list = _parse_add_ons_input(add_ons)

expected_files = FILES_IN_TEMPLATE_WITH_NO_ADD_ONS

for add_on in add_ons_list:
Expand Down Expand Up @@ -285,13 +285,14 @@ def test_parse_add_ons_invalid_range(input, capsys):


@pytest.mark.parametrize(
"input,first_invalid",
[("0,3,5", "0"), ("1,3,8", "8"), ("0-4", "0"), ("3-8", "8")],
"input,last_invalid",
[("0,3,5", "0"), ("1,3,8", "8"), ("0-4", "0"), ("3-9", "9")],
)
def test_parse_add_ons_invalid_selection(input, first_invalid, capsys):
AhdraMeraliQB marked this conversation as resolved.
Show resolved Hide resolved
def test_parse_add_ons_invalid_selection(input, last_invalid, capsys):
with pytest.raises(SystemExit):
_parse_add_ons_input(input)
message = f"'{first_invalid}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7."
selected = _parse_add_ons_input(input)
_validate_selection(selected)
message = f"'{last_invalid}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7."
assert message in capsys.readouterr().err


Expand Down Expand Up @@ -876,7 +877,22 @@ def test_directory_flag_with_starter_alias(self, fake_kedro_cli):
class TestAddOnsFromUserPrompts:
@pytest.mark.parametrize(
"add_ons",
["1", "2", "3", "4", "5", "6", "7", "none", "2,3,4", "3-5", "all"],
[
"1",
"2",
"3",
"4",
"5",
"6",
"7",
"none",
"2,3,4",
"3-5",
"all",
"1, 2, 3",
" 1, 2, 3 ",
"ALL",
],
)
def test_valid_add_ons(self, fake_kedro_cli, add_ons):
result = CliRunner().invoke(
Expand All @@ -889,11 +905,15 @@ def test_valid_add_ons(self, fake_kedro_cli, add_ons):
_assert_requirements_ok(result, add_ons=add_ons)
_clean_up_project(Path("./new-kedro-project"))

def test_invalid_add_ons(self, fake_kedro_cli):
@pytest.mark.parametrize(
"bad_input",
["bad input", "-1", "3-"],
)
def test_invalid_add_ons(self, fake_kedro_cli, bad_input):
result = CliRunner().invoke(
fake_kedro_cli,
["new"],
input=_make_cli_prompt_input(add_ons="bad input"),
input=_make_cli_prompt_input(add_ons=bad_input),
)

assert result.exit_code != 0
Expand All @@ -903,12 +923,57 @@ def test_invalid_add_ons(self, fake_kedro_cli):
in result.output
)

@pytest.mark.parametrize(
"input,last_invalid",
[("0,3,5", "0"), ("1,3,9", "9"), ("0-4", "0"), ("3-9", "9"), ("99", "99")],
)
def test_invalid_add_ons_selection(self, fake_kedro_cli, input, last_invalid):
result = CliRunner().invoke(
fake_kedro_cli,
["new"],
input=_make_cli_prompt_input(add_ons=input),
)

assert result.exit_code != 0
message = f"'{last_invalid}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7."
assert message in result.output

@pytest.mark.parametrize(
"input",
["5-2", "3-1"],
)
def test_invalid_add_ons_range(self, fake_kedro_cli, input):
result = CliRunner().invoke(
fake_kedro_cli,
["new"],
input=_make_cli_prompt_input(add_ons=input),
)

assert result.exit_code != 0
message = f"'{input}' is an invalid range for project add-ons.\nPlease ensure range values go from smaller to larger."
assert message in result.output


@pytest.mark.usefixtures("chdir_to_tmp")
class TestAddOnsFromConfigFile:
@pytest.mark.parametrize(
"add_ons",
["1", "2", "3", "4", "5", "6", "7", "none", "2,3,4", "3-5", "all"],
[
"1",
"2",
"3",
"4",
"5",
"6",
"7",
"none",
"2,3,4",
"3-5",
"all",
"1, 2, 3",
" 1, 2, 3 ",
"ALL",
],
)
def test_valid_add_ons(self, fake_kedro_cli, add_ons):
"""Test project created from config."""
Expand All @@ -927,10 +992,14 @@ def test_valid_add_ons(self, fake_kedro_cli, add_ons):
_assert_requirements_ok(result, add_ons=add_ons, repo_name="new-kedro-project")
_clean_up_project(Path("./new-kedro-project"))

def test_invalid_add_ons(self, fake_kedro_cli):
@pytest.mark.parametrize(
"bad_input",
["bad input", "-1", "3-"],
)
def test_invalid_add_ons(self, fake_kedro_cli, bad_input):
"""Test project created from config."""
config = {
"add_ons": "bad input",
"add_ons": bad_input,
"project_name": "My Project",
"repo_name": "my-project",
"python_package": "my_project",
Expand All @@ -947,6 +1016,46 @@ def test_invalid_add_ons(self, fake_kedro_cli):
in result.output
)

@pytest.mark.parametrize(
"input,last_invalid",
[("0,3,5", "0"), ("1,3,9", "9"), ("0-4", "0"), ("3-9", "9"), ("99", "99")],
)
def test_invalid_add_ons_selection(self, fake_kedro_cli, input, last_invalid):
config = {
"add_ons": input,
"project_name": "My Project",
"repo_name": "my-project",
"python_package": "my_project",
}
_write_yaml(Path("config.yml"), config)
result = CliRunner().invoke(
fake_kedro_cli, ["new", "-v", "--config", "config.yml"]
)

assert result.exit_code != 0
message = f"'{last_invalid}' is not a valid selection.\nPlease select from the available add-ons: 1, 2, 3, 4, 5, 6, 7."
assert message in result.output

@pytest.mark.parametrize(
"input",
["5-2", "3-1"],
)
def test_invalid_add_ons_range(self, fake_kedro_cli, input):
config = {
"add_ons": input,
"project_name": "My Project",
"repo_name": "my-project",
"python_package": "my_project",
}
_write_yaml(Path("config.yml"), config)
result = CliRunner().invoke(
fake_kedro_cli, ["new", "-v", "--config", "config.yml"]
)

assert result.exit_code != 0
message = f"'{input}' is an invalid range for project add-ons.\nPlease ensure range values go from smaller to larger."
assert message in result.output


@pytest.mark.usefixtures("chdir_to_tmp")
class TestAddOnsFromCLI:
Expand All @@ -967,9 +1076,14 @@ class TestAddOnsFromCLI:
"log, docs, data, test, lint",
"log, docs, data, test, lint",
"all",
"LINT",
"ALL",
"NONE",
"TEST, LOG, DOCS",
"test, DATA, liNt",
],
)
def test_valid_add_ons(self, fake_kedro_cli, add_ons):
def test_valid_add_ons_flag(self, fake_kedro_cli, add_ons):
result = CliRunner().invoke(
fake_kedro_cli,
["new", "--addons", add_ons],
Expand All @@ -980,7 +1094,7 @@ def test_valid_add_ons(self, fake_kedro_cli, add_ons):
_assert_requirements_ok(result, add_ons=add_ons, repo_name="new-kedro-project")
_clean_up_project(Path("./new-kedro-project"))

def test_invalid_add_ons(self, fake_kedro_cli):
def test_invalid_add_ons_flag(self, fake_kedro_cli):
result = CliRunner().invoke(
fake_kedro_cli,
["new", "--addons", "bad_input"],
Expand All @@ -997,7 +1111,7 @@ def test_invalid_add_ons(self, fake_kedro_cli):
"add_ons",
["lint,all", "test,none", "all,none"],
)
def test_invalid_add_on_combination(self, fake_kedro_cli, add_ons):
def test_invalid_add_ons_flag_combination(self, fake_kedro_cli, add_ons):
result = CliRunner().invoke(
fake_kedro_cli,
["new", "--addons", add_ons],
Expand Down