From d6a78f41eefb9664d2cc41ce8db16c2e71e61e9b Mon Sep 17 00:00:00 2001 From: Dmitry Sorokin <40151847+DimedS@users.noreply.github.com> Date: Thu, 18 Jan 2024 15:05:24 +0000 Subject: [PATCH] Change to short names for tools in config file (#3499) * Change to short names for tools in config file --------- Signed-off-by: Dmitry Sorokin Signed-off-by: Dmitry Sorokin <40151847+DimedS@users.noreply.github.com> Co-authored-by: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com> --- RELEASE.md | 1 + docs/source/starters/index.md | 4 +- docs/source/starters/new_project_tools.md | 2 +- features/steps/cli_steps.py | 6 +- features/tools.feature | 12 +- kedro/framework/cli/starters.py | 220 +++++++-------- tests/framework/cli/test_starters.py | 321 ++++++++-------------- 7 files changed, 233 insertions(+), 333 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 22ab2ea0c6..1f41509f3f 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -12,6 +12,7 @@ ## Breaking changes to the API * Added logging about not using async mode in `SequentiallRunner` and `ParallelRunner`. +* Changed input format for tools option obtained from --config file from numbers to short names. ## Documentation changes * Added documentation about `bootstrap_project` and `configure_project`. diff --git a/docs/source/starters/index.md b/docs/source/starters/index.md index 4cdb5a2d74..7033965cd4 100644 --- a/docs/source/starters/index.md +++ b/docs/source/starters/index.md @@ -32,8 +32,8 @@ The configuration file must contain: Additionally, the configuration file may contain: -* `tools`: The tools to customise your project setup with. Select from comma-separated values, ranges, or 'all/none'. Omitting this from your configuration file will result in the default selection of `none`. -* `example`: Indicate `yes` or `no` to select whether you would like your project to be populated with example code. Omitting this from your configuration file will result in the default selection of `no`. +* `tools`: The tools to customise your project setup with. Select from comma-separated values `lint, test, log, docs, data, pyspark, viz` or `all/none`. Omitting this from your configuration file will result in the default selection of `none`. +* `example_pipeline`: Indicate `yes` or `no` to select whether you would like your project to be populated with example code. Omitting this from your configuration file will result in the default selection of `no`. The `output_dir` can be specified as `~` for the home directory or `.` for the current working directory. Here is an example `config.yml`, which assumes that a directory named `~/code` already exists: diff --git a/docs/source/starters/new_project_tools.md b/docs/source/starters/new_project_tools.md index 96b5ed4ff7..8415d0dbc7 100644 --- a/docs/source/starters/new_project_tools.md +++ b/docs/source/starters/new_project_tools.md @@ -137,7 +137,7 @@ As an alternative to the interactive project creation workflow, you can also sup "my project" "tools": - "2-6" + "lint, test, log, docs, data, pyspark, viz" "example_pipeline": "y" diff --git a/features/steps/cli_steps.py b/features/steps/cli_steps.py index d3d4f9afd1..9fee9b56c1 100644 --- a/features/steps/cli_steps.py +++ b/features/steps/cli_steps.py @@ -171,7 +171,7 @@ def create_config_file_without_starter(context): context.root_project_dir = context.temp_dir / context.project_name context.package_name = context.project_name.replace("-", "_") config = { - "tools": "1-5", + "tools": "lint, test, log, docs, data", "project_name": context.project_name, "example_pipeline": "no", "repo_name": context.project_name, @@ -189,14 +189,12 @@ def create_config_file_with_tools(context, tools): It takes a custom tools list and sets example prompt to `y`. """ - tools_str = tools if tools != "none" else "" - context.config_file = context.temp_dir / "config.yml" context.project_name = "project-dummy" context.root_project_dir = context.temp_dir / context.project_name context.package_name = context.project_name.replace("-", "_") config = { - "tools": tools_str, + "tools": tools, "example_pipeline": "y", "project_name": context.project_name, "repo_name": context.project_name, diff --git a/features/tools.feature b/features/tools.feature index 8a18565df3..0643570720 100644 --- a/features/tools.feature +++ b/features/tools.feature @@ -9,9 +9,9 @@ Feature: New Kedro project with tools Then I should get a successful exit code Scenario: Create a new Kedro project with all tools except 'viz' and 'pyspark' - Given I have prepared a config file with tools "1,2,3,4,5" + Given I have prepared a config file with tools "lint, test, log, docs, data" When I run a non-interactive kedro new without starter - Then the expected tool directories and files should be created with "1,2,3,4,5" + Then the expected tool directories and files should be created with "lint, test, log, docs, data" Given I have installed the project dependencies When I execute the kedro command "run" Then I should get a successful exit code @@ -25,17 +25,17 @@ Feature: New Kedro project with tools Then I should get a successful exit code Scenario: Create a new Kedro project with only 'pyspark' tool - Given I have prepared a config file with tools "6" + Given I have prepared a config file with tools "pyspark" When I run a non-interactive kedro new without starter - Then the expected tool directories and files should be created with "6" + Then the expected tool directories and files should be created with "pyspark" Given I have installed the project dependencies When I execute the kedro command "run" Then I should get a successful exit code Scenario: Create a new Kedro project with only 'viz' tool - Given I have prepared a config file with tools "7" + Given I have prepared a config file with tools "viz" When I run a non-interactive kedro new without starter - Then the expected tool directories and files should be created with "7" + Then the expected tool directories and files should be created with "viz" Given I have installed the project dependencies When I execute the kedro command "run" Then I should get a successful exit code diff --git a/kedro/framework/cli/starters.py b/kedro/framework/cli/starters.py index 32c3db5a6f..5f364fe36b 100644 --- a/kedro/framework/cli/starters.py +++ b/kedro/framework/cli/starters.py @@ -149,7 +149,7 @@ def _validate_flag_inputs(flag_inputs: dict[str, Any]) -> None: ) -def _validate_regex(pattern_name: str, text: str) -> None: +def _validate_input_with_regex_pattern(pattern_name: str, input: str) -> None: VALIDATION_PATTERNS = { "yes_no": { "regex": r"(?i)^\s*(y|yes|n|no)\s*$", @@ -157,7 +157,7 @@ def _validate_regex(pattern_name: str, text: str) -> None: }, "project_name": { "regex": r"^[\w -]{2,}$", - "error_message": f"{text}' is an invalid value for project name. It must contain only alphanumeric symbols, spaces, underscores and hyphens and be at least 2 characters long", + "error_message": f"{input}' is an invalid value for project name. It must contain only alphanumeric symbols, spaces, underscores and hyphens and be at least 2 characters long", }, "tools": { "regex": r"""^( @@ -167,11 +167,11 @@ def _validate_regex(pattern_name: str, text: str) -> None: (\ *,\ *\d+(\ *-\ *\d+)?)* # D: any number of instances of: a comma followed by B and C, spaces allowed \ *)?) # E: zero or one instances of (B,C,D) as empty strings are also permissible $""", - "error_message": f"'{text}' is an invalid value for project tools. Please select valid options for tools using comma-separated values, ranges, or 'all/none'.", + "error_message": f"'{input}' is an invalid value for project tools. Please select valid options for tools using comma-separated values, ranges, or 'all/none'.", }, } - if not re.match(VALIDATION_PATTERNS[pattern_name]["regex"], text, flags=re.X): + if not re.match(VALIDATION_PATTERNS[pattern_name]["regex"], input, flags=re.X): click.secho( VALIDATION_PATTERNS[pattern_name]["error_message"], fg="red", @@ -207,28 +207,26 @@ def _validate_selected_tools(selected_tools: str | None) -> None: def _print_selection_and_prompt_info( - selected_tools: str | None, example_pipeline: str | None, interactive: bool + selected_tools: str, example_pipeline: str, interactive: bool ) -> None: # Confirm tools selection - if selected_tools is not None: - if selected_tools == "['None']": - click.secho( - "You have selected no project tools", - fg="green", - ) - else: - click.secho( - f"You have selected the following project tools: {selected_tools}", - fg="green", - ) + if selected_tools == "['None']": + click.secho( + "You have selected no project tools", + fg="green", + ) + else: + click.secho( + f"You have selected the following project tools: {selected_tools}", + fg="green", + ) # Confirm example selection - if example_pipeline is not None: - if example_pipeline: - click.secho( - "It has been created with an example pipeline.", - fg="green", - ) + if example_pipeline == "True": + click.secho( + "It has been created with an example pipeline.", + fg="green", + ) # Give hint for skipping interactive flow if interactive: @@ -352,8 +350,8 @@ def new( # noqa: PLR0913 # If interactive flow used, print hint interactive_flow = prompts_required and not config_path _print_selection_and_prompt_info( - extra_context.get("tools"), - extra_context.get("example_pipeline"), + extra_context["tools"], + extra_context["example_pipeline"], interactive_flow, ) @@ -444,11 +442,11 @@ def _get_prompts_required_and_clear_from_CLI_provided( del prompts_required["tools"] if project_name is not None: - _validate_regex("project_name", project_name) + _validate_input_with_regex_pattern("project_name", project_name) del prompts_required["project_name"] if example_pipeline is not None: - _validate_regex("yes_no", example_pipeline) + _validate_input_with_regex_pattern("yes_no", example_pipeline) del prompts_required["example_pipeline"] return prompts_required @@ -522,7 +520,8 @@ def _get_extra_context( # noqa: PLR0913 starter_alias: str | None, ) -> dict[str, str]: """Generates a config dictionary that will be passed to cookiecutter as `extra_context`, based - on CLI flags, user prompts, or a configuration file. + on CLI flags, user prompts, configuration file or Default values. + It is crucial to return a dictionary with string values, otherwise, there will be issues with Cookiecutter. Args: prompts_required: a dictionary of all the prompts that will be shown to @@ -540,64 +539,51 @@ def _get_extra_context( # noqa: PLR0913 None in case the flag wasn't used Returns: - the prompts_required dictionary, with all the redundant information removed. + Config dictionary, passed the necessary processing, with default values if needed. """ if config_path: - extra_context = _fetch_config_from_file(config_path) - _validate_config_file_against_prompts(extra_context, prompts_required) - _validate_config_file_inputs(extra_context, starter_alias) + extra_context = _fetch_validate_parse_config_from_file( + config_path, prompts_required, starter_alias + ) else: - extra_context = _fetch_config_from_user_prompts( + extra_context = _fetch_validate_parse_config_from_user_prompts( prompts_required, cookiecutter_context ) - # Format - extra_context.setdefault("kedro_version", version) - - converted_tools = _convert_tool_names_to_numbers(selected_tools) - - if converted_tools is not None: - extra_context["tools"] = converted_tools - + # Update extra_context, if CLI inputs are available + if selected_tools is not None: + tools_numbers = _convert_tool_short_names_to_numbers(selected_tools) + extra_context["tools"] = _convert_tool_numbers_to_readable_names(tools_numbers) if project_name is not None: extra_context["project_name"] = project_name + if example_pipeline is not None: + extra_context["example_pipeline"] = str(_parse_yes_no_to_bool(example_pipeline)) - # Map the selected tools lists to readable name - tools_context = extra_context.get("tools") - tools = _parse_tools_input(tools_context) - - # Check if no tools selected - if not tools: - extra_context["tools"] = str(["None"]) - else: - extra_context["tools"] = str([NUMBER_TO_TOOLS_NAME[tool] for tool in tools]) - - extra_context["example_pipeline"] = _parse_yes_no_to_bool( - example_pipeline - if example_pipeline is not None - else extra_context.get("example_pipeline", "no") - ) + # set defaults for required fields, will be used mostly for starters + extra_context.setdefault("kedro_version", version) + extra_context.setdefault("tools", str(["None"])) + extra_context.setdefault("example_pipeline", "False") return extra_context -def _convert_tool_names_to_numbers(selected_tools: str | None) -> str | None: - """Prepares tools selection from the CLI input to the correct format +def _convert_tool_short_names_to_numbers(selected_tools: str) -> list: + """Prepares tools selection from the CLI or config input to the correct format to be put in the project configuration, if it exists. Replaces tool strings with the corresponding prompt number. Args: - selected_tools: a string containing the value for the --tools flag, + selected_tools: a string containing the value for the --tools flag or config file, or None in case none were provided, i.e. lint,docs. Returns: String with the numbers corresponding to the desired tools, or None in case the --tools flag was not used. """ - if selected_tools is None or selected_tools.lower() == "none": - return None + if selected_tools.lower() == "none": + return [] if selected_tools.lower() == "all": - return ",".join(NUMBER_TO_TOOLS_NAME.keys()) + return list(NUMBER_TO_TOOLS_NAME.keys()) tools = [] for tool in selected_tools.lower().split(","): @@ -608,11 +594,29 @@ def _convert_tool_names_to_numbers(selected_tools: str | None) -> str | None: # Remove duplicates if any tools = sorted(list(set(tools))) - return ",".join(tools) + return tools + + +def _convert_tool_numbers_to_readable_names(tools_numbers: list) -> str: + """Transform the list of tool numbers into a list of readable names, using 'None' for empty lists. + Then, convert the result into a string format to prevent issues with Cookiecutter. + """ + tools_names = [NUMBER_TO_TOOLS_NAME[tool] for tool in tools_numbers] + if tools_names == []: + tools_names = ["None"] + return str(tools_names) -def _fetch_config_from_file(config_path: str) -> dict[str, str]: +def _fetch_validate_parse_config_from_file( + config_path: str, prompts_required: dict, starter_alias: str | None +) -> dict[str, str]: """Obtains configuration for a new kedro project non-interactively from a file. + Validates that: + 1. All keys specified in prompts_required are retrieved from the configuration. + 2. The options 'tools' and 'example_pipeline' are not used in the configuration when any starter option is selected. + 3. Variables sourced from the configuration file adhere to the expected format. + + Parse tools from short names to list of numbers Args: config_path: The path of the config.yml which should contain the data required @@ -628,7 +632,7 @@ def _fetch_config_from_file(config_path: str) -> dict[str, str]: """ try: with open(config_path, encoding="utf-8") as config_file: - config = yaml.safe_load(config_file) + config: dict[str, str] = yaml.safe_load(config_file) if KedroCliError.VERBOSE_ERROR: click.echo(config_path + ":") @@ -638,11 +642,30 @@ def _fetch_config_from_file(config_path: str) -> dict[str, str]: f"Failed to generate project: could not load config at {config_path}." ) from exc - # The return type defined is more specific than the "Any" type config return from yaml.safe_load - return config # type: ignore[no-any-return] + if starter_alias and ("tools" in config or "example_pipeline" in config): + raise KedroCliError( + "The --starter flag can not be used with `example_pipeline` and/or `tools` keys in the config file." + ) + + _validate_config_file_against_prompts(config, prompts_required) + + _validate_input_with_regex_pattern( + "project_name", config.get("project_name", "New Kedro Project") + ) + + example_pipeline = config.get("example_pipeline", "no") + _validate_input_with_regex_pattern("yes_no", example_pipeline) + config["example_pipeline"] = str(_parse_yes_no_to_bool(example_pipeline)) + + tools_short_names = config.get("tools", "none").lower() + _validate_selected_tools(tools_short_names) + tools_numbers = _convert_tool_short_names_to_numbers(tools_short_names) + config["tools"] = _convert_tool_numbers_to_readable_names(tools_numbers) + + return config -def _fetch_config_from_user_prompts( +def _fetch_validate_parse_config_from_user_prompts( prompts: dict[str, Any], cookiecutter_context: OrderedDict | None ) -> dict[str, str]: """Interactively obtains information from user prompts. @@ -678,6 +701,16 @@ def _fetch_config_from_user_prompts( if user_input: prompt.validate(user_input) config[variable_name] = user_input + + if "tools" in config: + # convert tools input to list of numbers and validate + tools_numbers = _parse_tools_input(config["tools"]) + _validate_tool_selection(tools_numbers) + config["tools"] = _convert_tool_numbers_to_readable_names(tools_numbers) + if "example_pipeline" in config: + example_pipeline_bool = _parse_yes_no_to_bool(config["example_pipeline"]) + config["example_pipeline"] = str(example_pipeline_bool) + return config @@ -724,11 +757,8 @@ def _make_cookiecutter_args_and_fetch_template( if directory: cookiecutter_args["directory"] = directory - # If 'tools' or 'example_pipeline' are not specified in prompts.yml, CLI or config.yml, - # default options will be used instead - # That can be when starter used or while loading from config.yml - tools: str | list = config.get("tools", []) - example_pipeline = config.get("example_pipeline", False) + tools = config["tools"] + example_pipeline = config["example_pipeline"] starter_path = "git+https://github.com/kedro-org/kedro-starters.git" if "PySpark" in tools and "Kedro Viz" in tools: @@ -743,8 +773,7 @@ def _make_cookiecutter_args_and_fetch_template( elif "Kedro Viz" in tools: # Use the spaceflights-pandas-viz starter if only Kedro Viz is chosen. cookiecutter_args["directory"] = "spaceflights-pandas-viz" - cookiecutter_args["checkout"] = version - elif example_pipeline: + elif example_pipeline == "True": # Use spaceflights-pandas starter if example was selected, but PySpark or Viz wasn't cookiecutter_args["directory"] = "spaceflights-pandas" cookiecutter_args["checkout"] = version @@ -768,7 +797,7 @@ def _validate_config_file_against_prompts( KedroCliError: If the config file is empty or does not contain all the keys required in prompts, or if the output_dir specified does not exist. """ - if config is None: + if not config: raise KedroCliError("Config file is empty.") additional_keys = {"tools": "none", "example_pipeline": "no"} missing_keys = set(prompts) - set(config) @@ -792,37 +821,7 @@ def _validate_config_file_against_prompts( ) -def _validate_config_file_inputs( - config: dict[str, str], starter_alias: str | None -) -> None: - """Checks that variables provided through the config file are of the expected format. This - validates the config provided by `kedro new --config` in a similar way to `prompts.yml` for starters. - Also validates that "tools" or "example_pipeline" options cannot be used in config when any starter option is - selected. - - Args: - config: The config as a dictionary - starter_alias: Starter alias if it was provided from CLI, otherwise None - - Raises: - SystemExit: If the provided variables are not properly formatted. - """ - if starter_alias and ("tools" in config or "example_pipeline" in config): - raise KedroCliError( - "The --starter flag can not be used with `example_pipeline` and/or `tools` keys in the config file." - ) - - _validate_regex("project_name", config.get("project_name", "New Kedro Project")) - - input_tools = config.get("tools", "none") - _validate_regex("tools", input_tools.lower()) - selected_tools = _parse_tools_input(input_tools) - _validate_selection(selected_tools) - - _validate_regex("yes_no", config.get("example_pipeline", "no")) - - -def _validate_selection(tools: list[str]) -> None: +def _validate_tool_selection(tools: list[str]) -> None: # 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 tool in tools[::-1]: @@ -847,6 +846,11 @@ def _validate_range(start: Any, end: Any) -> None: message = f"'{start}-{end}' is an invalid range for project tools.\nPlease ensure range values go from smaller to larger." click.secho(message, fg="red", err=True) sys.exit(1) + # safeguard to prevent passing of excessively large intervals that could cause freezing: + if int(end) > len(NUMBER_TO_TOOLS_NAME): + message = f"'{end}' is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." + click.secho(message, fg="red", err=True) + sys.exit(1) if not tools_str: return [] # pragma: no cover @@ -936,10 +940,6 @@ def validate(self, user_input: str) -> None: click.secho(self.error_message, fg="red", err=True) sys.exit(1) - if self.title == "Project Tools": - # Validate user input - _validate_selection(_parse_tools_input(user_input)) - # noqa: unused-argument def _remove_readonly( diff --git a/tests/framework/cli/test_starters.py b/tests/framework/cli/test_starters.py index 1e11c6e45b..f3aae47924 100644 --- a/tests/framework/cli/test_starters.py +++ b/tests/framework/cli/test_starters.py @@ -3,7 +3,6 @@ from __future__ import annotations import shutil -from collections import OrderedDict from pathlib import Path import pytest @@ -17,12 +16,12 @@ _OFFICIAL_STARTER_SPECS_DICT, TEMPLATE_PATH, KedroStarterSpec, - _convert_tool_names_to_numbers, - _fetch_config_from_user_prompts, + _convert_tool_short_names_to_numbers, + _fetch_validate_parse_config_from_user_prompts, _make_cookiecutter_args_and_fetch_template, _parse_tools_input, _parse_yes_no_to_bool, - _validate_selection, + _validate_tool_selection, ) FILES_IN_TEMPLATE_WITH_NO_TOOLS = 15 @@ -330,14 +329,24 @@ def test_parse_tools_invalid_range(self, input, capsys): message = f"'{input}' is an invalid range for project tools.\nPlease ensure range values go from smaller to larger." assert message in capsys.readouterr().err + @pytest.mark.parametrize( + "input,right_border", + [("3-9", "9"), ("3-10000", "10000")], + ) + def test_parse_tools_range_too_high(self, input, right_border, capsys): + with pytest.raises(SystemExit): + _parse_tools_input(input) + message = f"'{right_border}' is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." + assert message in capsys.readouterr().err + @pytest.mark.parametrize( "input,last_invalid", - [("0,3,5", "0"), ("1,3,8", "8"), ("0-4", "0"), ("3-9", "9")], + [("0,3,5", "0"), ("1,3,8", "8"), ("0-4", "0")], ) def test_parse_tools_invalid_selection(self, input, last_invalid, capsys): with pytest.raises(SystemExit): selected = _parse_tools_input(input) - _validate_selection(selected) + _validate_tool_selection(selected) message = f"'{last_invalid}' is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." assert message in capsys.readouterr().err @@ -450,43 +459,11 @@ def test_custom_prompt_for_essential_variable(self, fake_kedro_cli): ) _clean_up_project(Path("./my_custom_repo")) - def test_fetch_config_from_user_prompts_with_context(self, mocker): - required_prompts = { - "project_name": { - "title": "Project Name", - "text": "Please enter a name for your new project.", - }, - "tools": { - "title": "Project Tools", - "text": "These optional tools can help you apply software engineering best practices.", - }, - "example_pipeline": { - "title": "Example Pipeline", - "text": "Select whether you would like an example spaceflights pipeline included in your project.", - }, - } - cookiecutter_context = OrderedDict( - [ - ("project_name", "New Kedro Project"), - ("tools", "none"), - ("example_pipeline", "no"), - ] - ) - mocker.patch("cookiecutter.prompt.read_user_variable", return_value="none") - config = _fetch_config_from_user_prompts( - prompts=required_prompts, cookiecutter_context=cookiecutter_context - ) - assert config == { - "example_pipeline": "none", - "project_name": "none", - "tools": "none", - } - - def test_fetch_config_from_user_prompts_without_context(self): + def test_fetch_validate_parse_config_from_user_prompts_without_context(self): required_prompts = {} message = "No cookiecutter context available." with pytest.raises(Exception, match=message): - _fetch_config_from_user_prompts( + _fetch_validate_parse_config_from_user_prompts( prompts=required_prompts, cookiecutter_context=None ) @@ -1044,6 +1021,8 @@ class TestToolsAndExampleFromUserPrompts: "1, 2, 3", " 1, 2, 3 ", "ALL", + "none", + "", ], ) @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) @@ -1064,33 +1043,10 @@ def test_valid_tools_and_example( _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) _assert_requirements_ok(result, tools=tools) - assert "You have selected the following project tools:" in result.output - assert ( - "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" - in result.output - ) - _clean_up_project(Path("./new-kedro-project")) - - @pytest.mark.parametrize( - "tools", - [ - "none", - "", - ], - ) - @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) - def test_no_tools_and_example(self, fake_kedro_cli, tools, example_pipeline): - result = CliRunner().invoke( - fake_kedro_cli, - ["new"], - input=_make_cli_prompt_input( - tools=tools, example_pipeline=example_pipeline - ), - ) - - _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) - _assert_requirements_ok(result, tools=tools) - assert "You have selected no project tools" in result.output + if tools not in ("none", ""): + assert "You have selected the following project tools:" in result.output + else: + assert "You have selected no project tools" in result.output assert ( "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" in result.output @@ -1117,7 +1073,14 @@ def test_invalid_tools(self, fake_kedro_cli, bad_input): @pytest.mark.parametrize( "input,last_invalid", - [("0,3,5", "0"), ("1,3,9", "9"), ("0-4", "0"), ("3-9", "9"), ("99", "99")], + [ + ("0,3,5", "0"), + ("1,3,9", "9"), + ("0-4", "0"), + ("99", "99"), + ("3-9", "9"), + ("3-10000", "10000"), + ], ) def test_invalid_tools_selection(self, fake_kedro_cli, input, last_invalid): result = CliRunner().invoke( @@ -1179,24 +1142,28 @@ class TestToolsAndExampleFromConfigFile: @pytest.mark.parametrize( "tools", [ - "1", - "2", - "3", - "4", - "5", - "6", - "7", - "2,3,4", - "3-5", - "1,2,4-6", - "1,2,4-6,7", - "4-6,7", - "1, 2 ,4 - 6, 7", - "1-3,5-7", + "lint", + "test", + "tests", + "log", + "logs", + "docs", + "doc", + "data", + "pyspark", + "viz", + "tests,logs,doc", + "test,data,lint", + "log,docs,data,test,lint", + "log, docs, data, test, lint", + "log, docs, data, test, lint", "all", - "1, 2, 3", - " 1, 2, 3 ", + "LINT", "ALL", + "TEST, LOG, DOCS", + "test, DATA, liNt", + "none", + "NONE", ], ) @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) @@ -1221,34 +1188,15 @@ def test_valid_tools_and_example( fake_kedro_cli, ["new", "-v", "--config", "config.yml"] ) - _assert_template_ok(result, **config) - _assert_requirements_ok(result, tools=tools, repo_name="new-kedro-project") - assert "You have selected the following project tools:" in result.output - assert ( - "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" - not in result.output - ) - _clean_up_project(Path("./new-kedro-project")) - - @pytest.mark.parametrize("tools", ["none", "NONE", ""]) - @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) - def test_no_tools_and_example(self, fake_kedro_cli, tools, example_pipeline): - """Test project created from config.""" - config = { - "tools": tools, - "project_name": "New Kedro Project", - "example_pipeline": example_pipeline, - "repo_name": "new-kedro-project", - "python_package": "new_kedro_project", - } - _write_yaml(Path("config.yml"), config) - result = CliRunner().invoke( - fake_kedro_cli, ["new", "-v", "--config", "config.yml"] - ) + tools = _convert_tool_short_names_to_numbers(selected_tools=tools) + tools = ",".join(tools) if tools != [] else "none" - _assert_template_ok(result, **config) + _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) _assert_requirements_ok(result, tools=tools, repo_name="new-kedro-project") - assert "You have selected no project tools" in result.output + if tools not in ("none", "NONE", ""): + assert "You have selected the following project tools:" in result.output + else: + assert "You have selected no project tools" in result.output assert ( "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" not in result.output @@ -1257,7 +1205,7 @@ def test_no_tools_and_example(self, fake_kedro_cli, tools, example_pipeline): @pytest.mark.parametrize( "bad_input", - ["bad input", "-1", "3-", "1 1"], + ["bad input", "0,3,5", "1,3,9", "llint", "tes", "test, lin"], ) def test_invalid_tools(self, fake_kedro_cli, bad_input): """Test project created from config.""" @@ -1274,9 +1222,8 @@ def test_invalid_tools(self, fake_kedro_cli, bad_input): ) assert result.exit_code != 0 - assert "is an invalid value for project tools." in result.output assert ( - "Please select valid options for tools using comma-separated values, ranges, or 'all/none'.\n" + "Please select from the available tools: lint, test, log, docs, data, pyspark, viz, all, none" in result.output ) @@ -1308,32 +1255,11 @@ def test_invalid_tools_with_starter(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_tools_selection(self, fake_kedro_cli, input, last_invalid): - config = { - "tools": input, - "project_name": "My Project", - "example_pipeline": "no", - "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 tools: 1, 2, 3, 4, 5, 6, 7." - assert message in result.output - @pytest.mark.parametrize( "input", - ["5-2", "3-1"], + ["lint,all", "test,none", "all,none"], ) - def test_invalid_tools_range(self, fake_kedro_cli, input): + def test_invalid_tools_flag_combination(self, fake_kedro_cli, input): config = { "tools": input, "project_name": "My Project", @@ -1347,8 +1273,10 @@ def test_invalid_tools_range(self, fake_kedro_cli, input): ) assert result.exit_code != 0 - message = f"'{input}' is an invalid range for project tools.\nPlease ensure range values go from smaller to larger." - assert message in result.output + assert ( + "Tools options 'all' and 'none' cannot be used with other options" + in result.output + ) @pytest.mark.parametrize("example_pipeline", ["y", "n", "N", "YEs", " yeS "]) def test_valid_example(self, fake_kedro_cli, example_pipeline): @@ -1417,6 +1345,8 @@ class TestToolsAndExampleFromCLI: "ALL", "TEST, LOG, DOCS", "test, DATA, liNt", + "none", + "NONE", ], ) @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) @@ -1425,47 +1355,22 @@ def test_valid_tools_flag(self, fake_kedro_cli, tools, example_pipeline, mocker) "kedro.framework.cli.starters._make_cookiecutter_args_and_fetch_template", side_effect=mock_make_cookiecutter_args_and_fetch_template, ) - result = CliRunner().invoke( - fake_kedro_cli, - ["new", "--tools", tools, "--example", example_pipeline], - input=_make_cli_prompt_input_without_tools(), - ) - - tools = _convert_tool_names_to_numbers(selected_tools=tools) - if not tools: - tools = "" - - _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) - _assert_requirements_ok(result, tools=tools, repo_name="new-kedro-project") - assert "You have selected the following project tools:" in result.output - assert ( - "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" - in result.output - ) - _clean_up_project(Path("./new-kedro-project")) - @pytest.mark.parametrize( - "tools", - [ - "none", - "NONE", - ], - ) - @pytest.mark.parametrize("example_pipeline", ["Yes", "No"]) - def test_tools_flag_none(self, fake_kedro_cli, tools, example_pipeline): result = CliRunner().invoke( fake_kedro_cli, ["new", "--tools", tools, "--example", example_pipeline], input=_make_cli_prompt_input_without_tools(), ) - tools = _convert_tool_names_to_numbers(selected_tools=tools) - if not tools: - tools = "" + tools = _convert_tool_short_names_to_numbers(selected_tools=tools) + tools = ",".join(tools) if tools != [] else "none" _assert_template_ok(result, tools=tools, example_pipeline=example_pipeline) _assert_requirements_ok(result, tools=tools, repo_name="new-kedro-project") - assert "You have selected no project tools" in result.output + if tools not in ("none", "NONE"): + assert "You have selected the following project tools:" in result.output + else: + assert "You have selected no project tools" in result.output assert ( "To skip the interactive flow you can run `kedro new` with\nkedro new --name= --tools= --example=" in result.output @@ -1586,81 +1491,77 @@ def parse_yes_no_to_bool_responds_none(self): class TestValidateSelection: - def test_validate_selection_valid(self): + def test_validate_tool_selection_valid(self): tools = ["1", "2", "3", "4"] - assert _validate_selection(tools) is None + assert _validate_tool_selection(tools) is None - def test_validate_selection_invalid_single_tool(self, capsys): + def test_validate_tool_selection_invalid_single_tool(self, capsys): tools = ["8"] with pytest.raises(SystemExit): - _validate_selection(tools) + _validate_tool_selection(tools) message = "is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." assert message in capsys.readouterr().err - def test_validate_selection_invalid_multiple_tools(self, capsys): + def test_validate_tool_selection_invalid_multiple_tools(self, capsys): tools = ["8", "10", "15"] with pytest.raises(SystemExit): - _validate_selection(tools) + _validate_tool_selection(tools) message = "is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." assert message in capsys.readouterr().err - def test_validate_selection_mix_valid_invalid_tools(self, capsys): + def test_validate_tool_selection_mix_valid_invalid_tools(self, capsys): tools = ["1", "8", "3", "15"] with pytest.raises(SystemExit): - _validate_selection(tools) + _validate_tool_selection(tools) message = "is not a valid selection.\nPlease select from the available tools: 1, 2, 3, 4, 5, 6, 7." assert message in capsys.readouterr().err - def test_validate_selection_empty_list(self): + def test_validate_tool_selection_empty_list(self): tools = [] - assert _validate_selection(tools) is None + assert _validate_tool_selection(tools) is None class TestConvertToolNamesToNumbers: - def test_convert_tool_names_to_numbers_with_valid_tools(self): + def test_convert_tool_short_names_to_numbers_with_valid_tools(self): selected_tools = "lint,test,docs" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "1,2,4" - - def test_convert_tool_names_to_numbers_with_none(self): - result = _convert_tool_names_to_numbers(None) - assert result is None + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == ["1", "2", "4"] - def test_convert_tool_names_to_numbers_with_empty_string(self): + def test_convert_tool_short_names_to_numbers_with_empty_string(self): selected_tools = "" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == [] - def test_convert_tool_names_to_numbers_with_none_string(self): + def test_convert_tool_short_names_to_numbers_with_none_string(self): selected_tools = "none" - result = _convert_tool_names_to_numbers(selected_tools) - assert result is None + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == [] - def test_convert_tool_names_to_numbers_with_all_string(self): - result = _convert_tool_names_to_numbers("all") - assert result == "1,2,3,4,5,6,7" + def test_convert_tool_short_names_to_numbers_with_all_string(self): + result = _convert_tool_short_names_to_numbers("all") + assert result == ["1", "2", "3", "4", "5", "6", "7"] - def test_convert_tool_names_to_numbers_with_mixed_valid_invalid_tools(self): + def test_convert_tool_short_names_to_numbers_with_mixed_valid_invalid_tools(self): selected_tools = "lint,invalid_tool,docs" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "1,4" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == ["1", "4"] - def test_convert_tool_names_to_numbers_with_whitespace(self): + def test_convert_tool_short_names_to_numbers_with_whitespace(self): selected_tools = " lint , test , docs " - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "1,2,4" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == ["1", "2", "4"] - def test_convert_tool_names_to_numbers_with_case_insensitive_tools(self): + def test_convert_tool_short_names_to_numbers_with_case_insensitive_tools(self): selected_tools = "Lint,TEST,Docs" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "1,2,4" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == ["1", "2", "4"] - def test_convert_tool_names_to_numbers_with_invalid_tools(self): + def test_convert_tool_short_names_to_numbers_with_invalid_tools(self): selected_tools = "invalid_tool1,invalid_tool2" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == [] - def test_convert_tool_names_to_numbers_with_duplicates(self): + def test_convert_tool_short_names_to_numbers_with_duplicates(self): selected_tools = "lint,test,tests" - result = _convert_tool_names_to_numbers(selected_tools) - assert result == "1,2" + result = _convert_tool_short_names_to_numbers(selected_tools) + assert result == ["1", "2"]