diff --git a/.github/workflows/test-build-idf-apps.yml b/.github/workflows/test-build-idf-apps.yml index 08ea59b..a8d9449 100644 --- a/.github/workflows/test-build-idf-apps.yml +++ b/.github/workflows/test-build-idf-apps.yml @@ -34,11 +34,11 @@ jobs: path: dist/idf_build_apps-*.whl build-apps-on-idf-env: - if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ github.ref == 'refs/heads/main' || contains(github.event.pull_request.labels.*.name, 'test-old-idf-releases') }} needs: build-python-packages strategy: matrix: - idf-branch: [ release-v5.0, release-v5.1, release-v5.2 ] + idf-branch: [ release-v5.0, release-v5.1, release-v5.2, release-v5.3 ] runs-on: ubuntu-latest container: image: espressif/idf:${{ matrix.idf-branch }} diff --git a/idf_build_apps/app.py b/idf_build_apps/app.py index 28477f0..b8b3846 100644 --- a/idf_build_apps/app.py +++ b/idf_build_apps/app.py @@ -310,7 +310,7 @@ def build_log_path(self) -> str: return os.path.join(self.build_path, self.build_log_filename) # use a temp file if build log path is not specified - return os.path.join(self.build_path, f'.temp.build.{hash(self)}.log') + return os.path.join(self.build_path, '.temp.build.log') @computed_field # type: ignore @property @@ -559,6 +559,8 @@ def build( self._post_build() + self._finalize() + def _post_build(self) -> None: """Post build actions for failed/success builds""" if self.build_status not in ( @@ -623,9 +625,13 @@ def _post_build(self) -> None: for line in lines[-self.LOG_DEBUG_LINES :]: self._logger.error('%s', line) + def _finalize(self) -> None: + """ + Actions for real success builds + """ + if self.build_status != BuildStatus.SUCCESS: return - # Actions for real success builds # remove temp log file if self._is_build_log_path_temp: os.unlink(self.build_log_path) diff --git a/idf_build_apps/args.py b/idf_build_apps/args.py index ef2a5de..a0b9908 100644 --- a/idf_build_apps/args.py +++ b/idf_build_apps/args.py @@ -16,9 +16,9 @@ import os import re import typing as t -import warnings from copy import deepcopy from dataclasses import InitVar, asdict, dataclass, field, fields +from enum import Enum from . import SESSION_ARGS, setup_logging from .app import App @@ -37,6 +37,10 @@ LOGGER = logging.getLogger(__name__) +class _Field(Enum): + UNSET = 'UNSET' + + @dataclass class FieldMetadata: """ @@ -44,7 +48,7 @@ class FieldMetadata: Some fields are used in argparse while running :func:`add_args_to_parser`. :param description: description of the field - :param deprecated_by: name of the field that deprecates this field + :param deprecates: deprecates field names, used in argparse :param shorthand: shorthand for the argument, used in argparse :param action: action for the argument, used in argparse :param nargs: nargs for the argument, used in argparse @@ -56,7 +60,7 @@ class FieldMetadata: description: t.Optional[str] = None # when deprecated, the field description will be copied from the deprecated_by field if not specified - deprecated_by: t.Optional[str] = None + deprecates: t.Optional[t.Dict[str, t.Dict[str, t.Any]]] = None shorthand: t.Optional[str] = None # argparse_kwargs action: t.Optional[str] = None @@ -111,6 +115,17 @@ class GlobalArguments: ), ) + _depr_name_to_new_name_dict: t.ClassVar[t.Dict[str, str]] = {} # record deprecated field <-> new field + + def __new__(cls, *args, **kwargs): # noqa: ARG003 + for f in fields(cls): + _metadata = FieldMetadata(**f.metadata) + if _metadata.deprecates: + for depr_name in _metadata.deprecates: + cls._depr_name_to_new_name_dict[depr_name] = f.name + + return super().__new__(cls) + @classmethod def from_dict(cls, d: t.Dict[str, t.Any]) -> Self: """ @@ -119,45 +134,44 @@ def from_dict(cls, d: t.Dict[str, t.Any]) -> Self: :param d: dictionary :return: instance """ - return cls(**{k: v for k, v in d.items() if k in {f.name for f in fields(cls)}}) + return cls(**{k: v for k, v in d.items() if k in cls.__dataclass_fields__}) def __post_init__(self): self.apply_config() - def __setattr__(self, key, value): - _new_name_deprecated_name_dict = {} - _deprecated_name_new_name_dict = {} - for _n, _f in {f.name: f for f in fields(self)}.items(): - _meta = FieldMetadata(**_f.metadata) - if _meta.deprecated_by: - _new_name_deprecated_name_dict[_meta.deprecated_by] = _n - _deprecated_name_new_name_dict[_n] = _meta.deprecated_by - - if key in _new_name_deprecated_name_dict: - super().__setattr__(key, value) - # set together with the deprecated field - super().__setattr__(_new_name_deprecated_name_dict[key], value) - elif key in _deprecated_name_new_name_dict: - warnings.warn( - f'Field {key} is deprecated by {_deprecated_name_new_name_dict[key]}. Will be removed in the future.' - ) - super().__setattr__(key, value) - # set together with the new field - super().__setattr__(_deprecated_name_new_name_dict[key], value) - else: - super().__setattr__(key, value) - def apply_config(self) -> None: """ Apply the configuration file to the arguments """ - config_dict = get_valid_config(custom_path=self.config_file) + config_dict = get_valid_config(custom_path=self.config_file) or {} + + # set log fields first + self.verbose = config_dict.pop('verbose', self.verbose) + self.log_file = config_dict.pop('log_file', self.log_file) + self.no_color = config_dict.pop('no_color', self.no_color) + setup_logging(self.verbose, self.log_file, not self.no_color) + if config_dict: for name, value in config_dict.items(): if hasattr(self, name): setattr(self, name, value) - setup_logging(self.verbose, self.log_file, not self.no_color) + if name in self._depr_name_to_new_name_dict: + self.set_deprecated_field(self._depr_name_to_new_name_dict[name], name, value) + + def set_deprecated_field(self, new_k: str, depr_k: str, depr_v: t.Any) -> None: + if depr_v == _Field.UNSET: + return + + LOGGER.warning( + f'Field `{depr_k}` is deprecated. Will be removed in the next major release. ' + f'Use field `{new_k}` instead.' + ) + if getattr(self, new_k) is not None: + LOGGER.warning(f'Field `{new_k}` is already set. Ignoring deprecated field `{depr_k}`') + return + + setattr(self, new_k, depr_v) @dataclass @@ -166,19 +180,16 @@ class DependencyDrivenBuildArguments(GlobalArguments): Arguments used in the dependency-driven build feature. """ - manifest_file: t.Optional[t.List[str]] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='manifest_files', - nargs='+', - ) - ), - ) + manifest_file: InitVar[t.Optional[t.List[str]]] = _Field.UNSET manifest_files: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={ + 'manifest_file': { + 'nargs': '+', + }, + }, description='Path to the manifest files which contains the build test rules of the apps', nargs='+', ) @@ -211,20 +222,17 @@ class DependencyDrivenBuildArguments(GlobalArguments): ) ), ) - ignore_app_dependencies_components: t.Optional[t.List[str]] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='deactivate_dependency_driven_build_by_components', - type=semicolon_separated_str_to_list, - shorthand='-ic', - ) - ), - ) + ignore_app_dependencies_components: InitVar[t.Optional[t.List[str]]] = _Field.UNSET deactivate_dependency_driven_build_by_components: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={ + 'ignore_app_dependencies_components': { + 'type': semicolon_separated_str_to_list, + 'shorthand': '-ic', + } + }, description='semicolon-separated list of components. ' 'dependency-driven build feature will be deactivated when any of these components are modified', type=semicolon_separated_str_to_list, @@ -232,20 +240,17 @@ class DependencyDrivenBuildArguments(GlobalArguments): ) ), ) - ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='deactivate_dependency_driven_build_by_filepatterns', - type=semicolon_separated_str_to_list, - shorthand='-if', - ) - ), - ) + ignore_app_dependencies_filepatterns: InitVar[t.Optional[t.List[str]]] = _Field.UNSET deactivate_dependency_driven_build_by_filepatterns: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={ + 'ignore_app_dependencies_filepatterns': { + 'type': semicolon_separated_str_to_list, + 'shorthand': '-if', + } + }, description='semicolon-separated list of file patterns. ' 'dependency-driven build feature will be deactivated when any of matched files are modified', type=semicolon_separated_str_to_list, @@ -273,9 +278,26 @@ class DependencyDrivenBuildArguments(GlobalArguments): ), ) - def __post_init__(self): + def __post_init__( + self, + manifest_file: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_components: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ): super().__post_init__() + self.set_deprecated_field('manifest_files', 'manifest_file', manifest_file) + self.set_deprecated_field( + 'deactivate_dependency_driven_build_by_components', + 'ignore_app_dependencies_components', + ignore_app_dependencies_components, + ) + self.set_deprecated_field( + 'deactivate_dependency_driven_build_by_filepatterns', + 'ignore_app_dependencies_filepatterns', + ignore_app_dependencies_filepatterns, + ) + self.manifest_files = to_list(self.manifest_files) self.modified_components = to_list(self.modified_components) self.modified_files = to_list(self.modified_files) @@ -404,7 +426,7 @@ class FindBuildArguments(DependencyDrivenBuildArguments): ) ), ) - exclude_list: InitVar[t.Optional[t.List[str]]] = None + exclude_list: InitVar[t.Optional[t.List[str]]] = _Field.UNSET exclude: t.Optional[t.List[str]] = field( default=None, metadata=asdict( @@ -433,53 +455,36 @@ class FindBuildArguments(DependencyDrivenBuildArguments): ) ), ) - build_log: t.Optional[str] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='build_log_filename', - ) - ), - ) + build_log: InitVar[t.Optional[str]] = _Field.UNSET build_log_filename: t.Optional[str] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={'build_log': {}}, description='Log filename under the build directory instead of stdout. Can expand placeholders', ) ), ) - size_file: t.Optional[str] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='size_json_filename', - ) - ), - ) + size_file: InitVar[t.Optional[str]] = _Field.UNSET size_json_filename: t.Optional[str] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={'size_file': {}}, description='`idf.py size` output file under the build directory when specified. ' 'Can expand placeholders', ) ), ) - config: t.Optional[t.List[str]] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='config_rules', - nargs='+', - ) - ), - ) - config_rules_str: InitVar[t.Union[t.List[str], str, None]] = None + config: InitVar[t.Union[t.List[str], str, None]] = _Field.UNSET # cli # type: ignore + config_rules_str: InitVar[t.Union[t.List[str], str, None]] = _Field.UNSET # func # type: ignore config_rules: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={ + 'config': {'nargs': '+'}, + }, description='Defines the rules of building the project with pre-set sdkconfig files. ' 'Supports FILENAME[=NAME] or FILEPATTERN format. ' 'FILENAME is the filename of the sdkconfig file, relative to the app directory. ' @@ -573,16 +578,32 @@ class FindBuildArguments(DependencyDrivenBuildArguments): ), ) - def __post_init__( + def __post_init__( # type: ignore self, - exclude_list: t.Optional[t.List[str]] = None, - config_rules_str: t.Union[t.List[str], str, None] = None, + manifest_file: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_components: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + exclude_list: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + build_log: t.Optional[str] = _Field.UNSET, # type: ignore + size_file: t.Optional[str] = _Field.UNSET, # type: ignore + config: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + config_rules_str: t.Union[t.List[str], str, None] = _Field.UNSET, # type: ignore ): - super().__post_init__() + super().__post_init__( + manifest_file=manifest_file, + ignore_app_dependencies_components=ignore_app_dependencies_components, + ignore_app_dependencies_filepatterns=ignore_app_dependencies_filepatterns, + ) + + self.set_deprecated_field('exclude', 'exclude_list', exclude_list) + self.set_deprecated_field('build_log_filename', 'build_log', build_log) + self.set_deprecated_field('size_json_filename', 'size_file', size_file) + self.set_deprecated_field('config_rules', 'config', config) + self.set_deprecated_field('config_rules', 'config_rules_str', config_rules_str) self.paths = to_list(self.paths) - self.config_rules = to_list(self.config_rules) or to_list(config_rules_str) - self.exclude = self.exclude or exclude_list or [] + self.config_rules = to_list(self.config_rules) + self.exclude = to_list(self.exclude) # Validation if not self.paths: @@ -641,12 +662,27 @@ class FindArguments(FindBuildArguments): ), ) - def __post_init__( + def __post_init__( # type: ignore self, - exclude_list: t.Optional[t.List[str]] = None, - config_rules_str: t.Union[t.List[str], str, None] = None, + manifest_file: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_components: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + exclude_list: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + build_log: t.Optional[str] = _Field.UNSET, # type: ignore + size_file: t.Optional[str] = _Field.UNSET, # type: ignore + config: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + config_rules_str: t.Union[t.List[str], str, None] = _Field.UNSET, # type: ignore ): - super().__post_init__(exclude_list, config_rules_str) + super().__post_init__( + manifest_file=manifest_file, + ignore_app_dependencies_components=ignore_app_dependencies_components, + ignore_app_dependencies_filepatterns=ignore_app_dependencies_filepatterns, + exclude_list=exclude_list, + build_log=build_log, + size_file=size_file, + config=config, + config_rules_str=config_rules_str, + ) if self.include_all_apps: self.include_skipped_apps = True @@ -737,38 +773,26 @@ class BuildArguments(FindBuildArguments): ) ), ) - ignore_warning_str: t.Optional[t.List[str]] = field( - default=None, - metadata=asdict( - FieldMetadata( - deprecated_by='ignore_warning_strings', - nargs='+', - ) - ), - ) + ignore_warning_str: InitVar[t.Optional[t.List[str]]] = _Field.UNSET ignore_warning_strings: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={ + 'ignore_warning_str': {'nargs': '+'}, + }, description='space-separated list of patterns. ' 'Ignore the warnings in the build output that match the patterns', nargs='+', ) ), ) - ignore_warning_file: t.Optional[str] = field( - default=None, - metadata=asdict( - FieldMetadata( - description='Path to the file containing the patterns to ignore the warnings in the build output', - deprecated_by='ignore_warning_files', - ) - ), - ) + ignore_warning_file: InitVar[t.Optional[str]] = _Field.UNSET ignore_warning_files: t.Optional[t.List[str]] = field( default=None, metadata=asdict( FieldMetadata( + deprecates={'ignore_warning_file': {}}, description='Path to the files containing the patterns to ignore the warnings in the build output', nargs='+', ) @@ -792,12 +816,34 @@ class BuildArguments(FindBuildArguments): ), ) - def __post_init__( + def __post_init__( # type: ignore self, - exclude_list: t.Optional[t.List[str]] = None, - config_rules_str: t.Union[t.List[str], str, None] = None, + manifest_file: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_components: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + exclude_list: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + build_log: t.Optional[str] = _Field.UNSET, # type: ignore + size_file: t.Optional[str] = _Field.UNSET, # type: ignore + config: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + config_rules_str: t.Union[t.List[str], str, None] = _Field.UNSET, # type: ignore + ignore_warning_str: t.Optional[t.List[str]] = _Field.UNSET, # type: ignore + ignore_warning_file: t.Optional[str] = _Field.UNSET, # type: ignore ): - super().__post_init__(exclude_list, config_rules_str) + super().__post_init__( + manifest_file=manifest_file, + ignore_app_dependencies_components=ignore_app_dependencies_components, + ignore_app_dependencies_filepatterns=ignore_app_dependencies_filepatterns, + exclude_list=exclude_list, + build_log=build_log, + size_file=size_file, + config=config, + config_rules_str=config_rules_str, + ) + + self.set_deprecated_field('ignore_warning_strings', 'ignore_warning_str', ignore_warning_str) + self.set_deprecated_field('ignore_warning_files', 'ignore_warning_file', ignore_warning_file) + + self.ignore_warning_strings = to_list(self.ignore_warning_strings) or [] ignore_warnings_regexes = [] if self.ignore_warning_strings: @@ -868,13 +914,15 @@ def _drop_none(d: dict) -> dict: _meta = FieldMetadata(**f.metadata) desp = _meta.description - if _meta.deprecated_by: - if _meta.deprecated_by not in name_fields_dict: - raise ValueError(f'{_meta.deprecated_by} not found in {argument_cls}') - - deprecated_by_field = name_fields_dict[_meta.deprecated_by] - desp = desp or deprecated_by_field.metadata['description'] - desp = f'[DEPRECATED by {_snake_case_to_cli_arg_name(_meta.deprecated_by)}] {desp}' + # add deprecated fields + if _meta.deprecates: + for depr_k, depr_kwargs in _meta.deprecates.items(): + depr_kwargs['help'] = f'[DEPRECATED by {_snake_case_to_cli_arg_name(name)}] {desp}' + short_name = depr_kwargs.pop('shorthand', None) + _names = [_snake_case_to_cli_arg_name(depr_k)] + if short_name: + _names.append(short_name) + parser.add_argument(*_names, **depr_kwargs) # args args = [_snake_case_to_cli_arg_name(name)] diff --git a/idf_build_apps/finder.py b/idf_build_apps/finder.py index 75fa4eb..2d8b8b1 100644 --- a/idf_build_apps/finder.py +++ b/idf_build_apps/finder.py @@ -145,7 +145,7 @@ def _find_apps( if not args.recursive: if args.exclude: - LOGGER.warning('--exclude option is ignored when used without --recursive') + LOGGER.debug('--exclude option is ignored when used without --recursive') return _get_apps_from_path(path, target, app_cls=app_cls, args=args) diff --git a/idf_build_apps/log.py b/idf_build_apps/log.py index f0a9bab..6091da8 100644 --- a/idf_build_apps/log.py +++ b/idf_build_apps/log.py @@ -82,5 +82,7 @@ def setup_logging(verbose: int = 0, log_file: t.Optional[str] = None, colored: b handler = logging.StreamHandler(sys.stderr) handler.setFormatter(ColoredFormatter(colored)) + if package_logger.hasHandlers(): + package_logger.handlers.clear() package_logger.addHandler(handler) package_logger.propagate = False # don't propagate to root logger diff --git a/idf_build_apps/main.py b/idf_build_apps/main.py index bd68d6c..e62e732 100644 --- a/idf_build_apps/main.py +++ b/idf_build_apps/main.py @@ -344,7 +344,6 @@ def main(): if args.action == 'dump-manifest-sha': arguments = DumpManifestShaArguments.from_dict(vars(args)) - print(arguments) Manifest.from_files(arguments.manifest_files).dump_sha_values(arguments.output) sys.exit(0) diff --git a/idf_build_apps/manifest/manifest.py b/idf_build_apps/manifest/manifest.py index 39eb186..1531d0e 100644 --- a/idf_build_apps/manifest/manifest.py +++ b/idf_build_apps/manifest/manifest.py @@ -4,7 +4,6 @@ import os import pickle import typing as t -import warnings from hashlib import sha512 from pyparsing import ( @@ -257,7 +256,7 @@ def from_files(cls, paths: t.Iterable[str], *, root_path: str = os.curdir) -> 'M if cls.CHECK_MANIFEST_RULES: raise InvalidManifest(msg) else: - warnings.warn(msg) + LOGGER.warning(msg) _known_folders[rule.folder] = path @@ -290,7 +289,7 @@ def from_file(cls, path: str, *, root_path: str = os.curdir) -> 'Manifest': if cls.CHECK_MANIFEST_RULES: raise InvalidManifest(msg) else: - warnings.warn(msg) + LOGGER.warning(msg) try: rules.append(FolderRule(folder, **folder_rule if folder_rule else {})) diff --git a/pyproject.toml b/pyproject.toml index c7903c9..0466056 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,13 +28,11 @@ dependencies = [ "packaging", "toml; python_version < '3.11'", "pydantic~=2.0", - "argcomplete>=3" + "argcomplete>=3", + "typing-extensions; python_version < '3.11'", ] [project.optional-dependencies] -dev = [ - "typing-extensions; python_version < '3.8'", -] test = [ "pytest", "pytest-cov", diff --git a/tests/test_args.py b/tests/test_args.py new file mode 100644 index 0000000..c71ea94 --- /dev/null +++ b/tests/test_args.py @@ -0,0 +1,66 @@ +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 + + +from idf_build_apps.args import DependencyDrivenBuildArguments, FindArguments, FindBuildArguments +from idf_build_apps.config import IDF_BUILD_APPS_TOML_FN + + +def test_init_attr_deprecated_by(capsys): + args = DependencyDrivenBuildArguments( + ignore_app_dependencies_filepatterns=['bar'], + modified_files=['barfile'], + ) + assert args.deactivate_dependency_driven_build_by_filepatterns == ['bar'] + assert ( + 'Field `ignore_app_dependencies_filepatterns` is deprecated. Will be removed in the next major release. ' + 'Use field `deactivate_dependency_driven_build_by_filepatterns` instead' + ) in capsys.readouterr().err + + args = DependencyDrivenBuildArguments( + deactivate_dependency_driven_build_by_components=['foo'], + modified_components=['foocomp'], + ) + assert args.deactivate_dependency_driven_build_by_components == ['foo'] + # no warning this time + assert not capsys.readouterr().err + + +def test_init_attr_override(capsys): + args = FindBuildArguments( + config='foo', + config_rules_str='bar', + config_rules=['baz'], + ) + assert args.config_rules == ['baz'] + err_list = capsys.readouterr().err.splitlines() + assert len(err_list) == 4 + assert ( + 'Field `config` is deprecated. Will be removed in the next major release. ' 'Use field `config_rules` instead' + ) in err_list[0] + assert ('Field `config_rules` is already set. Ignoring deprecated field `config`') in err_list[1] + assert ( + 'Field `config_rules_str` is deprecated. Will be removed in the next major release. ' + 'Use field `config_rules` instead' + ) in err_list[2] + assert 'Field `config_rules` is already set. Ignoring deprecated field `config_rules_str`' in err_list[3] + + +def test_apply_config_with_deprecated_names(tmp_path, capsys): + with open(tmp_path / IDF_BUILD_APPS_TOML_FN, 'w') as fw: + fw.write("""config = [ + "foo" +] +no_color = true +""") + + args = FindBuildArguments(config_file=str(tmp_path / IDF_BUILD_APPS_TOML_FN)) + assert args.config_rules == ['foo'] + assert ( + 'Field `config` is deprecated. Will be removed in the next major release. ' 'Use field `config_rules` instead' + ) in capsys.readouterr().err + + +def test_empty_argument(): + args = FindArguments() + assert args.config_rules is None diff --git a/tests/test_finder.py b/tests/test_finder.py index 2b5067f..ccec111 100644 --- a/tests/test_finder.py +++ b/tests/test_finder.py @@ -25,7 +25,7 @@ class TestFindWithManifest: - def test_manifest_rootpath_chdir(self): + def test_manifest_rootpath_chdir(self, capsys): test_dir = Path(IDF_PATH) / 'examples' / 'get-started' yaml_file = test_dir / 'test.yml' @@ -40,13 +40,14 @@ def test_manifest_rootpath_chdir(self): os.chdir(IDF_PATH) assert not find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) + assert not capsys.readouterr().err # manifest folder invalid os.chdir(test_dir) - with pytest.warns(UserWarning, match=f'Folder "{test_dir}/examples/get-started" does not exist'): - assert find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) + assert find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) + assert f'Folder "{test_dir}/examples/get-started" does not exist' in capsys.readouterr().err - def test_manifest_rootpath_specified(self): + def test_manifest_rootpath_specified(self, capsys): test_dir = Path(IDF_PATH) / 'examples' / 'get-started' yaml_file = test_dir / 'test.yml' @@ -58,11 +59,12 @@ def test_manifest_rootpath_specified(self): """, encoding='utf8', ) - with pytest.warns(UserWarning, match=f'Folder "{IDF_PATH}/get-started" does not exist.'): - assert find_apps( - str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file), manifest_rootpath=str(IDF_PATH) - ) + assert find_apps( + str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file), manifest_rootpath=str(IDF_PATH) + ) + assert f'Folder "{IDF_PATH}/get-started" does not exist' in capsys.readouterr().err + # set correct rootpath assert not find_apps( str(test_dir), 'esp32', @@ -70,6 +72,7 @@ def test_manifest_rootpath_specified(self): manifest_files=str(yaml_file), manifest_rootpath=os.path.join(IDF_PATH, 'examples'), ) + assert not capsys.readouterr().err def test_keyword_idf_target(self, tmpdir): test_dir = os.path.join(IDF_PATH, 'examples') diff --git a/tests/test_manifest.py b/tests/test_manifest.py index bdeaaff..a70cfc2 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -10,6 +10,7 @@ ) import idf_build_apps +from idf_build_apps import setup_logging from idf_build_apps.constants import ( SUPPORTED_TARGETS, ) @@ -29,7 +30,9 @@ ) -def test_manifest(tmpdir, recwarn, monkeypatch): +def test_manifest_from_file_warning(tmpdir, capsys, monkeypatch): + setup_logging() + yaml_file = tmpdir / 'test.yml' yaml_file.write_text( """ @@ -49,12 +52,12 @@ def test_manifest(tmpdir, recwarn, monkeypatch): os.chdir(tmpdir) manifest = Manifest.from_file(yaml_file, root_path=tmpdir) + captured_err = capsys.readouterr().err.splitlines() msg_fmt = 'Folder "{}" does not exist. Please check your manifest file {}' - # two warnings warn test1 test2 not exists - assert len(recwarn) == 2 - assert recwarn.pop(UserWarning).message.args[0] == msg_fmt.format(os.path.join(tmpdir, 'test1'), yaml_file) - assert recwarn.pop(UserWarning).message.args[0] == msg_fmt.format(os.path.join(tmpdir, 'test2'), yaml_file) + assert len(captured_err) == 2 + assert msg_fmt.format(os.path.join(tmpdir, 'test1'), yaml_file) in captured_err[0] + assert msg_fmt.format(os.path.join(tmpdir, 'test2'), yaml_file) in captured_err[1] assert manifest.enable_build_targets('test1') == ['esp32', 'esp32c3', 'esp32s2'] assert manifest.enable_test_targets('test1') == ['esp32', 'esp32s2'] @@ -121,8 +124,7 @@ def test_manifest_switch_clause(tmpdir): ) os.chdir(tmpdir) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file, root_path=tmpdir) + manifest = Manifest.from_file(yaml_file) assert manifest.depends_components('test1', None, None) == ['VVV'] assert manifest.depends_components('test1', None, 'AAA') == ['VVV'] @@ -174,8 +176,7 @@ def test_manifest_switch_clause_with_postfix(tmpdir): encoding='utf8', ) os.chdir(tmpdir) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file, root_path=tmpdir) + manifest = Manifest.from_file(yaml_file, root_path=tmpdir) assert manifest.depends_components('test1', None, None) == ['DF'] assert manifest.depends_components('test1', None, 'CCC') == ['DF'] @@ -220,8 +221,7 @@ def test_manifest_switch_clause_wrong_manifest_format(tmpdir): encoding='utf8', ) try: - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - Manifest.from_file(yaml_file) + Manifest.from_file(yaml_file) except InvalidManifest as e: assert str(e) == 'Current manifest format has to fit either the switch format or the list format.' @@ -246,10 +246,7 @@ def test_manifest_with_anchor(tmpdir, monkeypatch): ) monkeypatch.setattr(idf_build_apps.manifest.manifest.FolderRule, 'DEFAULT_BUILD_TARGETS', ['esp32']) - - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file) - + manifest = Manifest.from_file(yaml_file) assert manifest.enable_build_targets('bar') == [] @@ -262,9 +259,7 @@ def test_manifest_with_anchor_and_postfix(tmpdir): """, encoding='utf8', ) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file) - + manifest = Manifest.from_file(yaml_file) assert manifest.enable_build_targets('foo') == sorted(SUPPORTED_TARGETS) yaml_file.write_text( @@ -285,9 +280,7 @@ def test_manifest_with_anchor_and_postfix(tmpdir): encoding='utf8', ) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file) - + manifest = Manifest.from_file(yaml_file) assert manifest.depends_components('examples/wifi/coexist') == ['esp_coex', 'esp_hw_support', 'esp_wifi'] yaml_file.write_text( @@ -434,9 +427,7 @@ def test_manifest_postfix_order(tmpdir): encoding='utf8', ) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - manifest = Manifest.from_file(yaml_file) - + manifest = Manifest.from_file(yaml_file) assert manifest.depends_components('examples/wifi/coexist') == ['esp_hw_support'] @@ -469,8 +460,7 @@ def test_from_files_duplicates(tmp_path, monkeypatch): Manifest.from_files([str(yaml_file_1), str(yaml_file_2)]) monkeypatch.setattr(idf_build_apps.manifest.manifest.Manifest, 'CHECK_MANIFEST_RULES', False) - with pytest.warns(UserWarning, match=f'Folder "{folder_path}" is already defined in {yaml_file_1!s}'): - Manifest.from_files([str(yaml_file_1), str(yaml_file_2)]) + Manifest.from_files([str(yaml_file_1), str(yaml_file_2)]) def test_manifest_dump_sha(tmpdir, sha_of_enable_only_esp32): @@ -487,8 +477,7 @@ def test_manifest_dump_sha(tmpdir, sha_of_enable_only_esp32): encoding='utf8', ) - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - Manifest.from_file(yaml_file).dump_sha_values(str(tmpdir / '.sha')) + Manifest.from_file(yaml_file).dump_sha_values(str(tmpdir / '.sha')) with open(tmpdir / '.sha') as f: assert f.readline() == f'bar:{sha_of_enable_only_esp32}\n' @@ -519,11 +508,10 @@ def test_manifest_diff_sha(tmpdir, sha_of_enable_only_esp32): fw.write(' ') # test spaces fw.write(f'foo:{sha_of_enable_only_esp32}\n') - with pytest.warns(UserWarning, match='Folder ".+" does not exist. Please check your manifest file'): - assert Manifest.from_file(yaml_file).diff_sha_with_filepath(str(tmpdir / '.sha')) == { - 'baz', - 'foo', - } + assert Manifest.from_file(yaml_file).diff_sha_with_filepath(str(tmpdir / '.sha')) == { + 'baz', + 'foo', + } class TestIfParser: