From 32a17dd444351931608ffc74b7b44953554e2bcf Mon Sep 17 00:00:00 2001 From: Dmitriy Astapov Date: Wed, 30 Oct 2024 16:48:27 +0100 Subject: [PATCH] feat(cli): add validations for CLI options --- idf_component_manager/cli/component.py | 21 +- idf_component_manager/cli/constants.py | 12 +- idf_component_manager/cli/manifest.py | 19 +- idf_component_manager/cli/project.py | 3 + idf_component_manager/cli/registry.py | 17 +- idf_component_manager/cli/validations.py | 158 +++++++++++++ idf_component_manager/core.py | 59 +---- idf_component_manager/idf_extensions.py | 15 +- idf_component_manager/utils.py | 13 -- idf_component_tools/file_tools.py | 8 +- integration_tests/README.md | 86 ++++---- integration_tests/test_add_dependency.py | 57 +++++ integration_tests/test_example_project.py | 37 +++- integration_tests/test_integration.py | 29 --- integration_tests/test_manifest.py | 30 +++ tests/cli/test_manifest_command.py | 14 +- tests/cli/test_validations.py | 208 ++++++++++++++++++ .../core/test_create_project_from_example.py | 28 +-- tests/core/test_pack_component.py | 56 ++++- 19 files changed, 687 insertions(+), 183 deletions(-) create mode 100644 idf_component_manager/cli/validations.py create mode 100644 integration_tests/test_add_dependency.py create mode 100644 integration_tests/test_manifest.py create mode 100644 tests/cli/test_validations.py diff --git a/idf_component_manager/cli/component.py b/idf_component_manager/cli/component.py index 885fc36e..d13f3706 100644 --- a/idf_component_manager/cli/component.py +++ b/idf_component_manager/cli/component.py @@ -3,6 +3,13 @@ import click +from idf_component_manager.cli.validations import ( + validate_git_url, + validate_if_archive, + validate_sha, + validate_version, +) + from .constants import ( get_dest_dir_option, get_name_option, @@ -41,11 +48,13 @@ def component(): '--repository', default=None, help='The URL of the component repository. This option overwrites the value in the idf_component.yml', + callback=validate_git_url, ), click.option( '--commit-sha', default=None, help='Git commit SHA of the the component version. This option overwrites the value in the idf_component.yml', + callback=validate_sha, ), click.option( '--repository-path', @@ -95,6 +104,7 @@ def pack( '--archive', help='Path of the archive with a component to upload. ' 'When not provided the component will be packed automatically.', + callback=validate_if_archive, ) @click.option( '--skip-pre-release', @@ -168,7 +178,9 @@ def upload_status(manager, profile_name, job): @component.command() @add_options(PROJECT_OPTIONS + NAMESPACE_NAME_OPTIONS) - @click.option('--version', required=True, help='Component version to delete.') + @click.option( + '--version', required=True, help='Component version to delete.', callback=validate_version + ) def delete(manager, profile_name, namespace, name, version): """ Delete specified version of the component from the component registry. @@ -178,7 +190,12 @@ def delete(manager, profile_name, namespace, name, version): @component.command() @add_options(PROJECT_OPTIONS + NAMESPACE_NAME_OPTIONS) - @click.option('--version', required=True, help='Component version to yank version.') + @click.option( + '--version', + required=True, + help='Component version to yank version.', + callback=validate_version, + ) @click.option( '--message', required=True, diff --git a/idf_component_manager/cli/constants.py b/idf_component_manager/cli/constants.py index 8ce8c90e..ad596215 100644 --- a/idf_component_manager/cli/constants.py +++ b/idf_component_manager/cli/constants.py @@ -6,8 +6,12 @@ import click from click.decorators import FC +from idf_component_manager.cli.validations import ( + combined_callback, + validate_existing_dir, + validate_name, +) from idf_component_manager.core import ComponentManager -from idf_component_manager.utils import validate_name def get_project_dir_option() -> t.List[FC]: @@ -16,7 +20,10 @@ def get_project_dir_option() -> t.List[FC]: '--project-dir', 'manager', default=os.getcwd(), - callback=lambda ctx, param, value: ComponentManager(value), # noqa: ARG005 + callback=combined_callback( + validate_existing_dir, + lambda ctx, param, value: ComponentManager(value), # noqa: ARG005 + ), ), ] @@ -47,6 +54,7 @@ def get_namespace_option() -> t.List[FC]: '--namespace', envvar='IDF_COMPONENT_NAMESPACE', default=None, + callback=validate_name, help='Namespace for the component. Can be set in config file.', ), ] diff --git a/idf_component_manager/cli/manifest.py b/idf_component_manager/cli/manifest.py index dd911ed6..b0e4dbf8 100644 --- a/idf_component_manager/cli/manifest.py +++ b/idf_component_manager/cli/manifest.py @@ -4,6 +4,12 @@ import click +from idf_component_manager.cli.validations import ( + validate_add_dependency, + validate_existing_dir, + validate_git_url, + validate_url, +) from idf_component_tools.manifest import MANIFEST_JSON_SCHEMA from .constants import get_profile_option, get_project_dir_option @@ -39,11 +45,14 @@ def schema(): '--path', default=None, help='Path to the component where the dependency will be added. The component name is ignored when the path is specified.', + callback=validate_existing_dir, ), ] GIT_OPTIONS = [ - click.option('--git', default=None, help='Git URL of the component.'), + click.option( + '--git', default=None, help='Git URL of the component.', callback=validate_git_url + ), click.option( '--git-path', default='.', help='Path to the component in the git repository.' ), @@ -78,9 +87,13 @@ def create(manager, component, path): + PROFILE_OPTION + MANIFEST_OPTIONS + GIT_OPTIONS - + [click.option('--registry-url', default=None, help='URL of the registry.')] + + [ + click.option( + '--registry-url', default=None, help='URL of the registry.', callback=validate_url + ) + ] ) - @click.argument('dependency', required=True) + @click.argument('dependency', required=True, callback=validate_add_dependency) def add_dependency( manager, profile_name, component, path, dependency, registry_url, git, git_path, git_ref ): diff --git a/idf_component_manager/cli/project.py b/idf_component_manager/cli/project.py index a90b5522..7e324ea7 100644 --- a/idf_component_manager/cli/project.py +++ b/idf_component_manager/cli/project.py @@ -2,6 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 import click +from idf_component_manager.cli.validations import validate_path_for_project + from .constants import get_project_dir_option, get_project_options from .utils import add_options @@ -25,6 +27,7 @@ def project(): default=None, help='Path of the new project. ' 'The project will be created directly in the given folder if it is empty.', + callback=validate_path_for_project, ) @click.argument('example', required=True) def create_from_example(manager, example, path, profile_name): diff --git a/idf_component_manager/cli/registry.py b/idf_component_manager/cli/registry.py index e94a32bf..ff607872 100644 --- a/idf_component_manager/cli/registry.py +++ b/idf_component_manager/cli/registry.py @@ -7,6 +7,12 @@ import click import requests +from idf_component_manager.cli.validations import ( + combined_callback, + validate_name, + validate_registry_component, + validate_url, +) from idf_component_manager.core import ComponentManager from idf_component_manager.utils import VersionSolverResolution from idf_component_tools import warn @@ -43,23 +49,25 @@ def registry(): @click.option( '--default-namespace', help='Default namespace to use for the components', + callback=validate_name, ) @click.option( '--default_namespace', help="This argument has been deprecated by 'default-namespace'", hidden=True, - callback=deprecated_option, + callback=combined_callback(deprecated_option, validate_name), expose_value=False, ) @click.option( '--registry-url', help='URL of the registry to use', + callback=validate_url, ) @click.option( '--registry_url', help="This argument has been deprecated by '--registry-url'", hidden=True, - callback=deprecated_option, + callback=combined_callback(deprecated_option, validate_url), expose_value=False, ) def login(profile_name, no_browser, description, default_namespace, registry_url): @@ -77,8 +85,8 @@ def login(profile_name, no_browser, description, default_namespace, registry_url # Check if token is already in the profile if profile.api_token: raise FatalError( - 'You are already logged in with profile "{}", ' - 'please either logout or use different profile'.format(profile_name) + f'You are already logged in with profile "{profile_name}", ' + 'please either logout or use a different profile' ) api_client = get_api_client( @@ -186,6 +194,7 @@ def logout(profile_name, no_revoke): help='Specify the components to sync from the registry. ' 'Use multiple --component options for multiple components. ' 'Format: namespace/name. Example: example/cmp==1.0.0', + callback=validate_registry_component, ) @click.option( '--resolution', diff --git a/idf_component_manager/cli/validations.py b/idf_component_manager/cli/validations.py new file mode 100644 index 00000000..c0527083 --- /dev/null +++ b/idf_component_manager/cli/validations.py @@ -0,0 +1,158 @@ +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import re +from pathlib import Path +from urllib.parse import urlparse + +import click + +from idf_component_manager.core_utils import COMPONENT_FULL_NAME_WITH_SPEC_REGEX +from idf_component_tools.archive_tools import ArchiveError, get_format_from_path +from idf_component_tools.constants import COMPILED_COMMIT_ID_RE, COMPILED_GIT_URL_RE +from idf_component_tools.manifest import WEB_DEPENDENCY_REGEX +from idf_component_tools.manifest.constants import SLUG_REGEX +from idf_component_tools.semver import Version +from idf_component_tools.semver.base import SimpleSpec + + +def validate_name(ctx, param, value): # noqa: ARG001 + if value is not None: + name = value.lower() + + if not re.match(SLUG_REGEX, name): + raise click.BadParameter( + f'"{name}" should consist of 2 or more letters, numbers, "-" or "_". ' + 'It cannot start or end with "_" or "-", or have sequences of these characters.' + ) + return name + + +def validate_existing_dir(ctx, param, value): # noqa: ARG001 + if value is not None: + if not value or not Path(value).is_dir(): + raise click.BadParameter(f'"{value}" directory does not exist.') + return value + + +def validate_url(ctx, param, value): # noqa: ARG001 + if value is not None: + result = urlparse(value) + if not result.scheme or not result.hostname: + raise click.BadParameter('Invalid URL.') + return value + + +def validate_sha(ctx, param, value): # noqa: ARG001 + if value is not None and not COMPILED_COMMIT_ID_RE.match(value): + raise click.BadParameter('Invalid SHA-1 hash.') + return value + + +def validate_git_url(ctx, param, value): # noqa: ARG001 + if value is not None and not COMPILED_GIT_URL_RE.match(value): + raise click.BadParameter('Invalid Git remote URL.') + return value + + +def validate_path_for_project(ctx, param, value): # noqa: ARG001 + if value is not None: + project_path = Path(value) + if project_path.is_file(): + raise click.BadParameter( + f'Your target path is not a directory. ' + f'Please remove the {project_path.resolve()} or use a different target path.' + ) + + if project_path.is_dir() and any(project_path.iterdir()): + raise click.BadParameter( + f'The directory "{project_path}" is not empty. ' + 'To create an example you must empty the directory or ' + 'choose a different path.', + ) + return value + + +def validate_if_archive(ctx, param, value): # noqa: ARG001 + if value is not None: + if not Path(value).is_file(): + raise click.BadParameter( + f'Cannot find archive to upload: {value}. Please check the path or if it exists.' + ) + try: + get_format_from_path(value) + except ArchiveError: + raise click.BadParameter(f'Unknown archive extension for file: {value}') + return value + + +def validate_version(ctx, param, value): # noqa: ARG001 + if value is not None: + try: + Version.parse(value) + except ValueError: + raise click.BadParameter( + f'Invalid version scheme.\n' + f'Received: "{value}"\n' + 'Documentation: https://docs.espressif.com/projects/idf-component-manager/en/' + 'latest/reference/versioning.html#versioning-scheme' + ) + return value + + +def validate_registry_component(ctx, param, value): # noqa: ARG001 + if value is not None: + for component in value: + match = re.match(COMPONENT_FULL_NAME_WITH_SPEC_REGEX, component) + if not match: + raise click.BadParameter( + 'Cannot parse COMPONENT argument. ' + 'Please use format like: namespace/component=1.0.0' + ) + + version_spec = match.group('version') or '*' + + try: + SimpleSpec(version_spec) + except ValueError: + raise click.BadParameter( + f'Invalid version specification: "{version_spec}". Please use format like ">=1" or "*".' + ) + return value + + +def validate_add_dependency(ctx, param, value): # noqa: ARG001 + if not value: + raise click.BadParameter('Name of the dependency can not be an empty string') + + if 'git' not in ctx.params: + match = re.match(WEB_DEPENDENCY_REGEX, value) + if match: + _, spec = match.groups() + else: + raise click.BadParameter( + f'Invalid dependency: "{value}". Please use format "namespace/name".' + ) + + if not spec: + spec = '*' + + try: + SimpleSpec(spec) + except ValueError: + raise click.BadParameter( + f'Invalid dependency version requirement: {spec}. ' + 'Please use format like ">=1" or "*".' + ) + + return value + + +# Function to combine multiple callback, order sensetive - each callback will be executed in the order in which it was passed +# If passed callback terminate command execution, it will terminate execution of loop as well +def combined_callback(*callbacks): + def wrapper(ctx, param, value): + for callback in callbacks: + value = callback(ctx, param, value) + return value + + return wrapper diff --git a/idf_component_manager/core.py b/idf_component_manager/core.py index f8899b15..f7d944ac 100644 --- a/idf_component_manager/core.py +++ b/idf_component_manager/core.py @@ -63,7 +63,7 @@ get_api_client, get_storage_client, ) -from idf_component_tools.semver import SimpleSpec, Version +from idf_component_tools.semver.base import Version from idf_component_tools.sources import GitSource, WebServiceSource from idf_component_tools.utils import ProjectRequirements @@ -158,7 +158,6 @@ def _get_manifest_dir(self, component: str = 'main', path: t.Optional[str] = Non raise FatalError( 'Cannot determine manifest directory. Please specify either component or path.' ) - # If path is specified if path is not None: manifest_dir = Path(path).resolve() @@ -221,21 +220,6 @@ def create_project_from_example( ) project_path = Path(path) if path else self.path / os.path.basename(example_name) - if project_path.is_file(): - raise FatalError( - 'Your target path is not a directory. ' - f'Please remove the {project_path.resolve()} or use different target path.', - exit_code=4, - ) - - if project_path.is_dir() and any(project_path.iterdir()): - raise FatalError( - f'The directory {project_path} is not empty. ' - 'To create an example you must empty the directory or ' - 'choose a different path.', - exit_code=3, - ) - component_details = client.component(component_name=component_name, version=version_spec) try: @@ -287,25 +271,13 @@ def add_dependency( GitSource(git=git, path=git_path).exists(git_ref) else: match = re.match(WEB_DEPENDENCY_REGEX, dependency) - if match: - name, spec = match.groups() - else: - raise FatalError( - f'Invalid dependency: "{dependency}". Please use format "namespace/name".' - ) + name, spec = match.groups() # type: ignore if not spec: spec = '*' - try: - SimpleSpec(spec) - except ValueError: - raise FatalError( - f'Invalid dependency version requirement: {spec}. ' - 'Please use format like ">=1" or "*".' - ) - name = WebServiceSource().normalized_name(name) + # Check if dependency exists in the registry # make sure it exists in the registry's storage url client = get_storage_client( @@ -537,9 +509,6 @@ def upload_component( api_client = get_api_client(namespace=namespace, profile_name=profile_name) if archive: - if not os.path.isfile(archive): - raise FatalError(f'Cannot find archive to upload: {archive}') - if version: raise FatalError( 'Parameters "version" and "archive" are not supported at the same time' @@ -775,22 +744,16 @@ def prepare_dep_dirs( for is_root, group in enumerate([downloaded_components, root_managed_components]): for downloaded_component in group: file.write( - 'idf_build_component("{}" "{}")\n'.format( - downloaded_component.abs_posix_path, - ('idf_components' if is_root == 1 else 'project_managed_components'), - ) + f'idf_build_component("{downloaded_component.abs_posix_path}" "{"idf_components" if is_root == 1 else "project_managed_components"}")\n' ) + file.write( f'idf_component_set_property({downloaded_component.name} COMPONENT_VERSION "{downloaded_component.version}")\n' ) if downloaded_component.targets: file.write( - 'idf_component_set_property({} {} "{}")\n'.format( - downloaded_component.name, - 'REQUIRED_IDF_TARGETS', - ' '.join(downloaded_component.targets), - ) + f'idf_component_set_property({downloaded_component.name} REQUIRED_IDF_TARGETS "{" ".join(downloaded_component.targets)}")\n' ) file.write( @@ -956,13 +919,9 @@ def _override_requirements_by_component_sources( == props['__COMPONENT_SOURCE'] ): raise RequirementsProcessingError( - 'Cannot process component requirements. ' - 'Requirement {} and requirement {} are both added as {}.' - "Can't decide which one to pick.".format( - name_matched_before.name, - comp_name.name, - props['__COMPONENT_SOURCE'], - ) + f'Cannot process component requirements. ' + f'Requirement {name_matched_before.name} and requirement {comp_name.name} are both added as {props["__COMPONENT_SOURCE"]}. ' + "Can't decide which one to pick." ) # Give user an info when same name components got overriden else: diff --git a/idf_component_manager/idf_extensions.py b/idf_component_manager/idf_extensions.py index 4fd959af..b97b6364 100755 --- a/idf_component_manager/idf_extensions.py +++ b/idf_component_manager/idf_extensions.py @@ -3,6 +3,12 @@ import sys import typing as t +from idf_component_manager.cli.validations import ( + validate_add_dependency, + validate_existing_dir, + validate_git_url, + validate_path_for_project, +) from idf_component_manager.utils import ( CLICK_SUPPORTS_SHOW_DEFAULT, ) @@ -42,6 +48,7 @@ 'names': ['--git'], 'help': 'Git URL of the component.', 'required': False, + 'callback': validate_git_url, }, { 'names': ['--git-path'], @@ -67,6 +74,7 @@ 'names': ['-p', '--path'], 'help': 'Path to the component where the dependency will be added. The component name is ignored when path the is specified.', 'default': None, + 'callback': validate_existing_dir, }, ] @@ -110,6 +118,10 @@ def callback(subcommand_name, ctx, args, **kwargs): # noqa: ARG001 error(str(e)) sys.exit(e.exit_code) + def add_dependency_callback(subcommand_name, ctx, args, **kwargs): + validate_add_dependency(ctx, None, kwargs.get('dependency')) + callback(subcommand_name, ctx, args, **kwargs) + def global_callback(ctx, global_args, tasks): # noqa: ARG001 copy_tasks = list(tasks) for index, task in enumerate(copy_tasks): @@ -142,7 +154,7 @@ def global_callback(ctx, global_args, tasks): # noqa: ARG001 'options': LOCAL_MANIFEST_OPTIONS, }, 'add-dependency': { - 'callback': callback, + 'callback': add_dependency_callback, 'help': ( 'Add dependency to the manifest file.\n' 'By default:\n' @@ -257,6 +269,7 @@ def global_callback(ctx, global_args, tasks): # noqa: ARG001 'if it does not contain anything' ), 'required': False, + 'callback': validate_path_for_project, } ], }, diff --git a/idf_component_manager/utils.py b/idf_component_manager/utils.py index b2c34c5f..a19b412c 100644 --- a/idf_component_manager/utils.py +++ b/idf_component_manager/utils.py @@ -2,28 +2,15 @@ # SPDX-License-Identifier: Apache-2.0 import enum -import re from enum import Enum import click -from idf_component_tools.manifest.constants import SLUG_REGEX from idf_component_tools.semver import Version CLICK_SUPPORTS_SHOW_DEFAULT = Version(click.__version__) >= Version('7.1.0') -def validate_name(ctx, param, value): # noqa: ARG001 - name = value.lower() - - if not re.match(SLUG_REGEX, name): - raise click.BadParameter( - f'"{name}" should consist of 2 or more letters, numbers, "-" or "_". ' - 'It cannot start or end with "_" or "-", or have sequences of these characters.' - ) - return name - - # total_ordering will raise an error in python 2.7 with enum34 # ValueError: must define at least one ordering operation: < > <= >= # The reason is that the `dir()` behavior is different, diff --git a/idf_component_tools/file_tools.py b/idf_component_tools/file_tools.py index 1f7cc5d8..883e37a4 100644 --- a/idf_component_tools/file_tools.py +++ b/idf_component_tools/file_tools.py @@ -9,6 +9,7 @@ from pathlib import Path from shutil import copytree, rmtree +from idf_component_tools.errors import FatalError from idf_component_tools.git_client import GitClient from idf_component_tools.messages import warn @@ -155,7 +156,12 @@ def prepare_empty_directory(directory: str) -> None: dir_exist = False if not dir_exist: - os.makedirs(directory) + try: + os.makedirs(directory) + except NotADirectoryError: + raise FatalError(f'Not a directory in the path. Cannot create directory: {directory}') + except PermissionError: + raise FatalError(f'Permission denied. Cannot create directory: {directory}') def copy_directory(source_directory: str, destination_directory: str) -> None: diff --git a/integration_tests/README.md b/integration_tests/README.md index c364a451..6a3decf8 100644 --- a/integration_tests/README.md +++ b/integration_tests/README.md @@ -1,35 +1,48 @@ # Integration test framework + The integration tests are configured to run in parallel sub-test groups with [pytest-split](https://pypi.org/project/pytest-split/). This `pytest` plugin enables dividing the test suite for a specific combination of ESP-IDF and Python versions into several test groups. The tests are evenly divided into groups by the plugin. ## Run integration tests locally + 1. Navigate to the root of this repository. -1. Install the Python dependencies: - ```sh - pip install '.[test]' - ``` -1. Install ESP-IDF. -1. Run `source ./export.sh` from the ESP-IDF root directory. -1. Navigate to the `integration_tests` directory. -1. Run the integration tests locally with the following command - ``` - pytest -c "../pytest_integration.ini" --log-cli-level=INFO - ``` +2. Install the Python dependencies: + ```sh + pip install '.[test]' + ``` +3. Install ESP-IDF. +4. Run `source ./export.sh` from the ESP-IDF root directory. +5. Navigate to the `integration_tests` directory. +6. Run the integration tests locally with the following command + ``` + pytest -c "../pytest_integration.ini" --log-cli-level=INFO + ``` + 1. To run tests from specific file run: + ``` + pytest file_name.py -c "../pytest_integration.ini" --log-cli-level=INFO + ``` + 2. To run specific test case run: + ``` + pytest -k 'name_of_test_case' -c "../pytest_integration.ini" --log-cli-level=INFO + ``` + ## Configure integration tests in the CI/CD pipeline Configure the Gitlab CI/CD pipeline with the `integration_tests.yml` pipeline definition file: 1. In the `parallel:matrix` job definition, add a new matrix dimension `PYTEST_SPLIT_TEST_GROUP`, and define a number of test groups to create. Use a number range starting from 1 to the desired number of groups, for example to split the test suite into 5 groups, use `PYTEST_SPLIT_TEST_GROUP: [1, 2, 3, 4, 5]`. - - **Note**: The `parallel:matrix` enables running test suites for a specific ESP-IDF development branch and a Python version in parallel jobs. So this pipeline implements two levels of parallelization. + + - **Note**: The `parallel:matrix` enables running test suites for a specific ESP-IDF development branch and a Python version in parallel jobs. So this pipeline implements two levels of parallelization. 2. Enter a number of groups in `` and configure the pipeline to run the following command. The number of the splits have to be the same as the number of the groups defined in the `PYTEST_SPLIT_TEST_GROUP` list: - ```sh - pytest -c "pytest_integration.ini" \ - --log-cli-level=INFO \ - --splits \ - --group ${PYTEST_SPLIT_TEST_GROUP} - ``` + + ```sh + pytest -c "pytest_integration.ini" \ + --log-cli-level=INFO \ + --splits \ + --group ${PYTEST_SPLIT_TEST_GROUP} + ``` ## Create an integration test @@ -40,7 +53,8 @@ structure. ```python @pytest.mark.parametrize( - 'project', [ + 'project', + [ { 'components': { 'component_name': { @@ -48,14 +62,15 @@ structure. 'some_component_dep': { 'git': 'https://github.com/espressif/esp-idf.git', 'path': 'components/some_component_dep/', - 'include': 'some_component_dep.h' + 'include': 'some_component_dep.h', } } } } } ], - indirect=True) + indirect=True, +) def test_single_dependency(project): assert some_action_with_project(project) ``` @@ -73,12 +88,9 @@ component dictionary. 'some_component_dep': { 'git': 'https://github.com/espressif/esp-idf.git', 'path': 'components/some_component_dep/', - 'include': 'some_component_dep.h' + 'include': 'some_component_dep.h', }, - 'some_component_dep2': { - 'version': '^1.0.0', - 'include': 'some_component_dep2.h' - } + 'some_component_dep2': {'version': '^1.0.0', 'include': 'some_component_dep2.h'}, }, 'cmake_lists': { 'priv_requires': 'another_component', @@ -89,10 +101,11 @@ component dictionary. ``` - `dependencies` - denotes on what components the component depends. - - `git` - denotes the url address for the git repository of the component (combine with `path`) - - `path` - denotes the path to the component from the root of the git repository - - `include`- value that is included in the source file of the component - - `version` - version of the component in the ESP Component Registry + + - `git` - denotes the url address for the git repository of the component (combine with `path`) + - `path` - denotes the path to the component from the root of the git repository + - `include`- value that is included in the source file of the component + - `version` - version of the component in the ESP Component Registry - `cmake_lists` - key-value in this dictionary will be used as the name of parameter and its value in the function `idf_component_register` of the `CMakeLists.txt`. @@ -130,7 +143,7 @@ tmp7F1Ssf 'unity': { 'git': 'https://github.com/espressif/esp-idf.git', 'path': 'components/unity/', - 'include': 'unity.h' + 'include': 'unity.h', } } } @@ -144,20 +157,13 @@ tmp7F1Ssf ```python { 'components': { - 'main': { - 'dependencies': { - 'mag3110': { - 'version': '^1.0.0', - 'include': 'mag3110.h' - } - } - } + 'main': {'dependencies': {'mag3110': {'version': '^1.0.0', 'include': 'mag3110.h'}}} } } ``` 3. The project contains two components - the main and "new_component". The "new_component" -privately requires the component button. This component is added into manifest of the main component + privately requires the component button. This component is added into manifest of the main component as a ESP Component Registry dependency. The `main.c` of the main component includes `new_component.h` and `button.h`. Test is successful when build of the project is successful. diff --git a/integration_tests/test_add_dependency.py b/integration_tests/test_add_dependency.py new file mode 100644 index 00000000..178580f8 --- /dev/null +++ b/integration_tests/test_add_dependency.py @@ -0,0 +1,57 @@ +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import os + +import pytest + +from .integration_test_helpers import ( + project_action, +) + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency(project): + res = project_action(project, 'add-dependency', 'example/cmp^3.3.8') + assert 'Successfully added dependency "example/cmp": "^3.3.8" to component "main"' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency_with_path(project): + path = os.path.join(project, 'project', 'src') + os.makedirs(path) + res = project_action(project, 'add-dependency', '--path', path, 'lvgl/lvgl>=8.*') + assert 'Successfully added dependency "lvgl/lvgl": ">=8.*" to component "src"' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency_with_not_existing_path(project): + path = os.path.join(project, 'not_existing_path') + res = project_action(project, 'add-dependency', '--path', path, 'lvgl/lvgl>=8.*') + assert f'"{path}" directory does not exist.' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency_invalid_dependency(project): + res = project_action(project, 'add-dependency', '/namespace//component/1.0.0') + assert ( + 'Invalid dependency: "/namespace//component/1.0.0". Please use format "namespace/name".' + in res + ) + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency_invalid_version(project): + res = project_action(project, 'add-dependency', 'namespace/component>=a.b.c') + assert 'Invalid dependency version requirement: >=a.b.c. ' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_add_dependency_invalid_git_url(project): + res = project_action( + project, + 'add-dependency', + '--git', + 'git_url', + 'cmp', + ) + assert 'Invalid Git remote UR' in res diff --git a/integration_tests/test_example_project.py b/integration_tests/test_example_project.py index e5d2328c..2dfdb82f 100644 --- a/integration_tests/test_example_project.py +++ b/integration_tests/test_example_project.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 from integration_tests.integration_test_helpers import project_action @@ -16,6 +16,41 @@ def test_create_project_from_example_path(tmp_path): assert (tmp_path / 'CMakeLists.txt').is_file() +def test_create_project_from_example_not_empty_path(tmp_path): + project_path = tmp_path / 'non_empty_dir' + project_path.mkdir() + # Add a file to make the directory non-empty + (project_path / 'example_file.txt').write_text('This is a test file.') + + res = project_action( + str(tmp_path), + 'create-project-from-example', + '-p', + str(project_path), + 'example/cmp=3.3.8:cmp_ex', + ) + + assert ( + f'''Invalid value for '-p' / '--path': The directory "{project_path}" is not empty. ''' + in res + ) + + +def test_create_project_from_example_path_is_file(tmp_path): + project_path = tmp_path / 'example_file.txt' + project_path.write_text('This is a test file.') + + res = project_action( + str(tmp_path), + 'create-project-from-example', + '-p', + str(project_path), + 'example/cmp=3.3.8:cmp_ex', + ) + + assert f'Your target path is not a directory. Please remove the {project_path}' in res + + def test_create_project_from_example_no_path(tmp_path): example_name = 'cmp_ex' res = project_action( diff --git a/integration_tests/test_integration.py b/integration_tests/test_integration.py index b5839a61..c13c158d 100644 --- a/integration_tests/test_integration.py +++ b/integration_tests/test_integration.py @@ -144,34 +144,6 @@ def test_root_dep_failed(project): assert 'component_foo' in res -@pytest.mark.parametrize('project', [{}], indirect=True) -def test_add_dependency(project): - res = project_action(project, 'add-dependency', 'example/cmp^3.3.8') - assert 'Successfully added dependency "example/cmp": "^3.3.8" to component "main"' in res - - -@pytest.mark.parametrize('project', [{}], indirect=True) -def test_add_dependency_with_path(project): - path = os.path.join(project, 'project', 'src') - os.makedirs(path) - res = project_action(project, 'add-dependency', '--path', path, 'lvgl/lvgl>=8.*') - assert 'Successfully added dependency "lvgl/lvgl": ">=8.*" to component "src"' in res - - -@pytest.mark.parametrize('project', [{}], indirect=True) -def test_create_manifest(project): - res = project_action(project, 'create-manifest') - path = os.path.join(project, 'main', 'idf_component.yml') - assert f'Created "{path}"' in res - - -@pytest.mark.parametrize('project', [{}], indirect=True) -def test_create_manifest_with_path(project): - res = project_action(project, 'create-manifest', '--path', project) - path = os.path.join(project, 'idf_component.yml') - assert f'Created "{path}"' in res - - @pytest.mark.parametrize( 'project', [ @@ -331,7 +303,6 @@ def test_idf_build_inject_dependencies_even_with_set_components( project_action(project, 'fullclean') # clean the downloaded component res = project_action(project, 'reconfigure') - print(res) with open(os.path.join(project, 'dependencies.lock')) as fr: lock = yaml.safe_load(fr) diff --git a/integration_tests/test_manifest.py b/integration_tests/test_manifest.py new file mode 100644 index 00000000..61cc4091 --- /dev/null +++ b/integration_tests/test_manifest.py @@ -0,0 +1,30 @@ +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import os + +import pytest + +from .integration_test_helpers import ( + project_action, +) + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_create_manifest(project): + res = project_action(project, 'create-manifest') + path = os.path.join(project, 'main', 'idf_component.yml') + assert f'Created "{path}"' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_create_manifest_with_path(project): + res = project_action(project, 'create-manifest', '--path', project) + path = os.path.join(project, 'idf_component.yml') + assert f'Created "{path}"' in res + + +@pytest.mark.parametrize('project', [{}], indirect=True) +def test_create_manifest_with_not_existing_path(project): + path = os.path.join(project, 'not_existing_path') + res = project_action(project, 'create-manifest', '--path', path) + assert f'"{path}" directory does not exist' in res diff --git a/tests/cli/test_manifest_command.py b/tests/cli/test_manifest_command.py index 54e0c568..51103cfc 100644 --- a/tests/cli/test_manifest_command.py +++ b/tests/cli/test_manifest_command.py @@ -152,7 +152,6 @@ def test_add_git_dependency(): 'https://github.com/espressif/example_components.git', ], ).output - assert 'Successfully' in result assert ( @@ -231,7 +230,7 @@ def test_add_git_dependency_invalid(): manager = ComponentManager(path=str(tempdir)) manager.create_manifest() - exception = runner.invoke( + output = runner.invoke( cli, [ 'manifest', @@ -241,12 +240,13 @@ def test_add_git_dependency_invalid(): 'https://github.com/espressif/example_compnents.git', ], ).exception + assert ( 'Repository "https://github.com/espressif/example_compnents.git" does not exist' - in str(exception) + in str(output) ) - exception = runner.invoke( + output = runner.invoke( cli, [ 'manifest', @@ -258,9 +258,9 @@ def test_add_git_dependency_invalid(): 'ciempi', ], ).exception - assert 'Path "ciempi" does not exist' in str(exception) + assert 'Path "ciempi" does not exist' in str(output) - exception = runner.invoke( + output = runner.invoke( cli, [ 'manifest', @@ -272,7 +272,7 @@ def test_add_git_dependency_invalid(): 'trest', ], ).exception - assert 'Git reference "trest" does not exist' in str(exception) + assert 'Git reference "trest" does not exist' in str(output) def test_manifest_keeps_comments(): diff --git a/tests/cli/test_validations.py b/tests/cli/test_validations.py new file mode 100644 index 00000000..81aed229 --- /dev/null +++ b/tests/cli/test_validations.py @@ -0,0 +1,208 @@ +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 + +import click +import pytest + +from idf_component_manager.cli.validations import ( + validate_add_dependency, + validate_existing_dir, + validate_git_url, + validate_if_archive, + validate_name, + validate_path_for_project, + validate_registry_component, + validate_sha, + validate_url, + validate_version, +) + +# Group of tests for callbacks used in click options +# TODO add non-failure cases + + +def test_validate_name(): + assert validate_name(None, None, 'valid') == 'valid' + + +def test_validate_name_invalid_input(): + with pytest.raises(click.BadParameter) as exc_info: + validate_name(None, None, 'a') + assert 'should consist of 2 or more letters, numbers' in str(exc_info.value) + + +def test_validate_existing_dir(tmp_path): + temp_dir = tmp_path / 'valid_dir' + temp_dir.mkdir() + assert validate_existing_dir(None, None, str(temp_dir)) == str(temp_dir) + + +@pytest.mark.parametrize( + 'invalid_dir', + [ + '/path/to/nonexistent/dir', # Non-existent directory + '', # Empty path + ], +) +def test_validate_existing_dir_invalid_input(invalid_dir): + with pytest.raises(click.BadParameter) as exc_info: + validate_existing_dir(None, None, invalid_dir) + assert f'"{invalid_dir}" directory does not exist.' in str(exc_info.value) + + +def test_validate_url(): + assert validate_url(None, None, 'https://hostname') == 'https://hostname' + + +@pytest.mark.parametrize( + 'invalid_url', + [ + 'invalid-url', # Missing scheme and netloc + 'http://', # Valid scheme but missing netloc + 'ftp://', # Unsupported scheme + 'http://:80', # Valid scheme but no host + 'www.example.com', # Missing scheme, + '', # Empty string + ], +) +def test_validate_url_invalid_input(invalid_url): + with pytest.raises(click.BadParameter) as exc_info: + validate_url(None, None, invalid_url) + assert 'Invalid URL.' in str(exc_info.value) + + +def test_validate_sha(): + assert validate_sha(None, None, 'a' * 40) == 'a' * 40 + + +def test_validate_sha_invalid_input(): + with pytest.raises(click.BadParameter) as exc_info: + validate_sha(None, None, 'g' * 40) # non-hexadecimal character + assert 'Invalid SHA-1 hash.' in str(exc_info.value) + + +def test_validate_git_url(): + assert ( + validate_git_url(None, None, 'https://github.com/username/repository.git') + == 'https://github.com/username/repository.git' + ) + + +def test_validate_git_url_invalid_input(): + with pytest.raises(click.BadParameter) as exc_info: + validate_git_url(None, None, 'github.com/repo') + assert 'Invalid Git remote URL.' in str(exc_info.value) + + +def test_validate_path_for_project(tmp_path): + temp_dir = tmp_path / 'empty_dir' + temp_dir.mkdir() + assert validate_path_for_project(None, None, str(temp_dir)) == str(temp_dir) + + +def test_validate_path_for_project_is_file(tmp_path): + temp_file = tmp_path / 'file.txt' + temp_file.touch() + with pytest.raises(click.BadParameter) as exc_info: + validate_path_for_project(None, None, str(temp_file)) + assert 'Your target path is not a directory. ' in str(exc_info.value) + + +def test_validate_path_for_project_not_empty_directory(tmp_path): + temp_dir = tmp_path / 'not_empty_directory' + temp_dir.mkdir() + (temp_dir / 'file.txt').touch() + with pytest.raises(click.BadParameter) as exc_info: + validate_path_for_project(None, None, str(temp_dir)) + assert f'The directory "{str(temp_dir)}" is not empty. ' in str(exc_info.value) + + +def test_validate_if_archive(tmp_path): + temp_file = tmp_path / 'file.zip' + temp_file.touch() + assert validate_if_archive(None, None, str(temp_file)) == str(temp_file) + + +@pytest.mark.parametrize( + 'invalid_archive', + [ + '/path/to/directory', # Path that is a directory, not a file + '', # Empty path + ], +) +def test_validate_if_archive_not_existing_file(invalid_archive): + with pytest.raises(click.BadParameter) as exc_info: + validate_if_archive(None, None, invalid_archive) + assert f'Cannot find archive to upload: {invalid_archive}' in str(exc_info.value) + + +def test_validate_if_archive_unknown_extension(tmp_path): + temp_file = tmp_path / 'file.uknown' + temp_file.touch() + with pytest.raises(click.BadParameter) as exc_info: + validate_if_archive(None, None, str(temp_file)) + assert f'Unknown archive extension for file: {str(temp_file)}' in str(exc_info.value) + + +def test_validate_version(): + assert validate_version(None, None, '1.2.3') == '1.2.3' + + +@pytest.mark.parametrize('version', ['1.2', '']) +def test_validate_version_invalid_input(version): + with pytest.raises(click.BadParameter) as exc_info: + validate_version(None, None, version) + assert 'Invalid version scheme.' in str(exc_info.value) + + +def test_validate_registry_component(): + assert validate_registry_component(None, None, ['namespace/component/1.2.3']) == [ + 'namespace/component/1.2.3' + ] + + +def test_validate_registry_component_invalid_component(): + with pytest.raises(click.BadParameter) as exc_info: + validate_registry_component(None, None, ['/namespace//component/1.0.0']) + assert 'Cannot parse COMPONENT argument. ' in str(exc_info.value) + + +def test_validate_registry_component_invalid_version(): + with pytest.raises(click.BadParameter) as exc_info: + validate_registry_component(None, None, ['test/test=1.a']) + assert 'Invalid version specification:' in str(exc_info.value) + + +def test_validate_add_dependency(): + ctx = click.Context(click.Command('add-dependency')) + assert ( + validate_add_dependency(ctx, None, 'namespace/component==1.2.3') + == 'namespace/component==1.2.3' + ) + ctx.params = {'git': ''} + assert validate_add_dependency(ctx, None, 'name') == 'name' + + +def test_validate_add_dependency_invalid_dependency(): + ctx = click.Context(click.Command('add-dependency')) + with pytest.raises(click.BadParameter) as exc_info: + validate_add_dependency(ctx, None, '/namespace//component/1.0.0') + assert ( + 'Invalid dependency: "/namespace//component/1.0.0". Please use format "namespace/name".' + in str(exc_info.value) + ) + + +def test_validate_add_dependency_invalid_version(): + ctx = click.Context(click.Command('add-dependency')) + with pytest.raises(click.BadParameter) as exc_info: + validate_add_dependency(ctx, None, 'namespace/component>=a.b.c') + assert 'Invalid dependency version requirement: >=a.b.c. ' in str(exc_info.value) + + +def test_validate_add_dependency_with_git_empty_name(): + ctx = click.Context(click.Command('add-dependency')) + ctx.params = {'git': ''} + with pytest.raises(click.BadParameter) as exc_info: + validate_add_dependency(ctx, None, '') + assert 'Name of the dependency can not be an empty string' in str(exc_info.value) diff --git a/tests/core/test_create_project_from_example.py b/tests/core/test_create_project_from_example.py index 184f1630..847af98a 100644 --- a/tests/core/test_create_project_from_example.py +++ b/tests/core/test_create_project_from_example.py @@ -7,28 +7,6 @@ from idf_component_tools.errors import FatalError -def test_create_example_project_path_not_a_directory(tmp_path): - existing_file = tmp_path / 'example' - existing_file.write_text('test') - - manager = ComponentManager(path=str(tmp_path)) - - with raises(FatalError, match='Your target path is not a directory*'): - manager.create_project_from_example('test:example') - - -def test_create_example_project_path_not_empty(tmp_path): - example_dir = tmp_path / 'example' - example_dir.mkdir() - existing_file = example_dir / 'test' - existing_file.write_text('test') - - manager = ComponentManager(path=str(tmp_path)) - - with raises(FatalError, match='To create an example you must*'): - manager.create_project_from_example('test:example') - - @vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_component_not_exist.yaml') def test_create_example_component_not_exist(tmp_path): manager = ComponentManager(path=str(tmp_path)) @@ -37,7 +15,7 @@ def test_create_example_component_not_exist(tmp_path): @vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_not_exist.yaml') -def test_create_example_version_not_exist(mock_registry, tmp_path): +def test_create_example_version_not_exist(mock_registry, tmp_path): # noqa: ARG001 manager = ComponentManager(path=str(tmp_path)) with raises( FatalError, @@ -47,7 +25,7 @@ def test_create_example_version_not_exist(mock_registry, tmp_path): @vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_not_exist.yaml') -def test_create_example_not_exist(mock_registry, tmp_path): +def test_create_example_not_exist(mock_registry, tmp_path): # noqa: ARG001 manager = ComponentManager(path=str(tmp_path)) with raises( FatalError, @@ -57,6 +35,6 @@ def test_create_example_not_exist(mock_registry, tmp_path): @vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_success.yaml') -def test_create_example_success(mock_registry, tmp_path): +def test_create_example_success(mock_registry, tmp_path): # noqa: ARG001 manager = ComponentManager(path=str(tmp_path)) manager.create_project_from_example('test/cmp>=1.0.0:sample_project') diff --git a/tests/core/test_pack_component.py b/tests/core/test_pack_component.py index d043981a..294e2368 100644 --- a/tests/core/test_pack_component.py +++ b/tests/core/test_pack_component.py @@ -7,7 +7,9 @@ import pytest import yaml +from click.testing import CliRunner +from idf_component_manager.cli.core import initialize_cli from idf_component_manager.core import ComponentManager from idf_component_tools.archive_tools import unpack_archive from idf_component_tools.constants import MANIFEST_FILENAME @@ -80,20 +82,44 @@ def test_pack_component_no_version_provided_nor_manifest(tmp_path, release_compo def test_pack_component_version_from_git(monkeypatch, tmp_path, pre_release_component_path): + monkeypatch.setenv('IDF_TOOLS_PATH', str(tmp_path)) copy_into(pre_release_component_path, str(tmp_path)) component_manager = ComponentManager(path=str(tmp_path)) # remove the first version line remove_version_line(tmp_path) - def mock_git_tag(self, cwd=None): + def mock_git_tag(self, cwd=None): # noqa: ARG001 return Version('3.0.0') monkeypatch.setattr(GitClient, 'get_tag_version', mock_git_tag) - component_manager.pack_component('pre', 'git') + # Define a destination directory within tmp_path + dist_dir = tmp_path / 'dist' + dist_dir.mkdir(parents=True, exist_ok=True) + + runner = CliRunner() + cli = initialize_cli() + result = runner.invoke( + cli, + [ + 'component', + 'pack', + '--version', + 'git', + '--name', + 'pre', + '--dest-dir', + str(dist_dir), + '--project-dir', + str(tmp_path), + ], + ) + + assert result.exit_code == 0 tempdir = os.path.join(tempfile.tempdir, 'cmp_pre') + unpack_archive(os.path.join(component_manager.default_dist_path, 'pre_3.0.0.tgz'), tempdir) manifest = ManifestManager(tempdir, 'pre').load() assert manifest.version == '3.0.0' @@ -117,9 +143,11 @@ def mock_git_tag(self, cwd=None): ('2.3.4~1', '2.3.4~1'), ], ) -def test_pack_component_with_dest_dir(version, expected_version, tmp_path, release_component_path): +def test_pack_component_with_dest_dir( + monkeypatch, version, expected_version, tmp_path, release_component_path +): + monkeypatch.setenv('IDF_TOOLS_PATH', str(tmp_path)) copy_into(release_component_path, str(tmp_path)) - component_manager = ComponentManager(path=str(tmp_path)) dest_path = tmp_path / 'dest_dir' os.mkdir(str(dest_path)) @@ -127,7 +155,25 @@ def test_pack_component_with_dest_dir(version, expected_version, tmp_path, relea # remove the first version line remove_version_line(tmp_path) - component_manager.pack_component('cmp', version, 'dest_dir') + runner = CliRunner() + cli = initialize_cli() + result = runner.invoke( + cli, + [ + 'component', + 'pack', + '--version', + version, + '--name', + 'cmp', + '--dest-dir', + str(dest_path), + '--project-dir', + str(tmp_path), + ], + ) + + assert result.exit_code == 0 tempdir = os.path.join(tempfile.tempdir, 'cmp') unpack_archive(os.path.join(str(dest_path), 'cmp_{}.tgz'.format(expected_version)), tempdir)