From 2d094ab9d323838a09d8bbe95281ac5a91231be0 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Fri, 26 Jan 2024 14:10:46 +0100 Subject: [PATCH 1/3] test: remove succeeded temp projects also rename `tmp_path` to `work_dirpath` since it's defined in pytest --- tools/ci/idf_pytest/tests/conftest.py | 15 +++++- .../ci/idf_pytest/tests/test_get_all_apps.py | 50 +++++++++---------- .../idf_pytest/tests/test_get_pytest_cases.py | 26 +++++----- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/tools/ci/idf_pytest/tests/conftest.py b/tools/ci/idf_pytest/tests/conftest.py index b2244bd5b6..bd85b7f4df 100644 --- a/tools/ci/idf_pytest/tests/conftest.py +++ b/tools/ci/idf_pytest/tests/conftest.py @@ -1,8 +1,11 @@ # SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 +import logging import os +import shutil import sys import tempfile +import typing as t from pathlib import Path import pytest @@ -53,7 +56,15 @@ void app_main(void) {} @pytest.fixture -def tmp_path() -> Path: +def work_dirpath() -> t.Generator[Path, None, None]: os.makedirs(os.path.join(IDF_PATH, 'pytest_embedded_log'), exist_ok=True) - return Path(tempfile.mkdtemp(prefix=os.path.join(IDF_PATH, 'pytest_embedded_log') + os.sep)) + p = Path(tempfile.mkdtemp(prefix=os.path.join(IDF_PATH, 'pytest_embedded_log') + os.sep)) + + try: + yield p + except Exception: + logging.critical('Test is failing, Please check the log in %s', p) + raise + else: + shutil.rmtree(p) diff --git a/tools/ci/idf_pytest/tests/test_get_all_apps.py b/tools/ci/idf_pytest/tests/test_get_all_apps.py index d4d53baee8..6996c4d5bb 100644 --- a/tools/ci/idf_pytest/tests/test_get_all_apps.py +++ b/tools/ci/idf_pytest/tests/test_get_all_apps.py @@ -8,19 +8,19 @@ from idf_pytest.script import SUPPORTED_TARGETS from conftest import create_project -def test_get_all_apps_non(tmp_path: Path) -> None: - create_project('foo', tmp_path) - create_project('bar', tmp_path) +def test_get_all_apps_non(work_dirpath: Path) -> None: + create_project('foo', work_dirpath) + create_project('bar', work_dirpath) - test_related_apps, non_test_related_apps = get_all_apps([str(tmp_path)]) + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)]) assert test_related_apps == set() assert len(non_test_related_apps) == 2 * len(SUPPORTED_TARGETS) -def test_get_all_apps_single_dut_test_script(tmp_path: Path) -> None: - create_project('foo', tmp_path) - with open(tmp_path / 'foo' / 'pytest_get_all_apps_single_dut_test_script.py', 'w') as fw: +def test_get_all_apps_single_dut_test_script(work_dirpath: Path) -> None: + create_project('foo', work_dirpath) + with open(work_dirpath / 'foo' / 'pytest_get_all_apps_single_dut_test_script.py', 'w') as fw: fw.write( """import pytest @@ -30,18 +30,18 @@ def test_foo(dut): pass """ ) - create_project('bar', tmp_path) + create_project('bar', work_dirpath) - test_related_apps, non_test_related_apps = get_all_apps([str(tmp_path)], target='all') + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') assert len(test_related_apps) == 2 assert len(non_test_related_apps) == 2 * len(SUPPORTED_TARGETS) - 2 -def test_get_all_apps_multi_dut_with_markers_test_script(tmp_path: Path) -> None: - create_project('foo', tmp_path) +def test_get_all_apps_multi_dut_with_markers_test_script(work_dirpath: Path) -> None: + create_project('foo', work_dirpath) - (tmp_path / 'foo' / 'pytest_get_all_apps_multi_dut_with_markers_test_script.py').write_text( + (work_dirpath / 'foo' / 'pytest_get_all_apps_multi_dut_with_markers_test_script.py').write_text( """import pytest @pytest.mark.esp32 @@ -52,15 +52,15 @@ def test_foo(dut): encoding='utf-8', ) - test_related_apps, non_test_related_apps = get_all_apps([str(tmp_path)], target='all') + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') assert len(test_related_apps) == 1 assert len(non_test_related_apps) == len(SUPPORTED_TARGETS) - 1 -def test_get_all_apps_multi_dut_test_script(tmp_path: Path) -> None: - create_project('foo', tmp_path) - with open(tmp_path / 'foo' / 'pytest_get_all_apps_multi_dut_test_script.py', 'w') as fw: +def test_get_all_apps_multi_dut_test_script(work_dirpath: Path) -> None: + create_project('foo', work_dirpath) + with open(work_dirpath / 'foo' / 'pytest_get_all_apps_multi_dut_test_script.py', 'w') as fw: fw.write( """import pytest @@ -75,17 +75,17 @@ def test_foo(dut): """ ) - test_related_apps, non_test_related_apps = get_all_apps([str(tmp_path)], target='all') + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') assert len(test_related_apps) == 3 # 32, s2, s3 assert len(non_test_related_apps) == len(SUPPORTED_TARGETS) - 3 -def test_get_all_apps_modified_pytest_script(tmp_path: Path) -> None: - create_project('foo', tmp_path) - create_project('bar', tmp_path) +def test_get_all_apps_modified_pytest_script(work_dirpath: Path) -> None: + create_project('foo', work_dirpath) + create_project('bar', work_dirpath) - (tmp_path / 'pytest_get_all_apps_modified_pytest_script.py').write_text( + (work_dirpath / 'pytest_get_all_apps_modified_pytest_script.py').write_text( """import pytest import os @@ -100,20 +100,20 @@ def test_multi_foo_bar(dut): encoding='utf-8', ) - test_related_apps, non_test_related_apps = get_all_apps([str(tmp_path)], target='all') + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') assert len(test_related_apps) == 2 # foo-esp32, bar-esp32 assert len(non_test_related_apps) == 2 * len(SUPPORTED_TARGETS) - 2 test_related_apps, non_test_related_apps = get_all_apps( - [str(tmp_path)], target='all', modified_files=[], modified_components=[] + [str(work_dirpath)], target='all', modified_files=[], modified_components=[] ) assert len(test_related_apps) == 0 assert len(non_test_related_apps) == 0 test_related_apps, non_test_related_apps = get_all_apps( - [str(tmp_path)], + [str(work_dirpath)], target='all', - modified_files=[str(tmp_path / 'pytest_get_all_apps_modified_pytest_script.py')], + modified_files=[str(work_dirpath / 'pytest_get_all_apps_modified_pytest_script.py')], modified_components=[], ) assert len(test_related_apps) == 2 diff --git a/tools/ci/idf_pytest/tests/test_get_pytest_cases.py b/tools/ci/idf_pytest/tests/test_get_pytest_cases.py index e221fef935..f74607194c 100644 --- a/tools/ci/idf_pytest/tests/test_get_pytest_cases.py +++ b/tools/ci/idf_pytest/tests/test_get_pytest_cases.py @@ -32,41 +32,41 @@ def test_foo_multi_with_marker(dut): ''' -def test_get_pytest_cases_single_specific(tmp_path: Path) -> None: - script = tmp_path / 'pytest_get_pytest_cases_single_specific.py' +def test_get_pytest_cases_single_specific(work_dirpath: Path) -> None: + script = work_dirpath / 'pytest_get_pytest_cases_single_specific.py' script.write_text(TEMPLATE_SCRIPT) - cases = get_pytest_cases([str(tmp_path)], 'esp32') + cases = get_pytest_cases([str(work_dirpath)], 'esp32') assert len(cases) == 1 assert cases[0].targets == ['esp32'] -def test_get_pytest_cases_multi_specific(tmp_path: Path) -> None: - script = tmp_path / 'pytest_get_pytest_cases_multi_specific.py' +def test_get_pytest_cases_multi_specific(work_dirpath: Path) -> None: + script = work_dirpath / 'pytest_get_pytest_cases_multi_specific.py' script.write_text(TEMPLATE_SCRIPT) - cases = get_pytest_cases([str(tmp_path)], 'esp32s2,esp32s2, esp32s3') + cases = get_pytest_cases([str(work_dirpath)], 'esp32s2,esp32s2, esp32s3') assert len(cases) == 1 assert cases[0].targets == ['esp32s2', 'esp32s2', 'esp32s3'] - cases = get_pytest_cases([str(tmp_path)], 'esp32s3,esp32s2,esp32s2') # order matters + cases = get_pytest_cases([str(work_dirpath)], 'esp32s3,esp32s2,esp32s2') # order matters assert len(cases) == 0 -def test_get_pytest_cases_multi_all(tmp_path: Path) -> None: - script = tmp_path / 'pytest_get_pytest_cases_multi_all.py' +def test_get_pytest_cases_multi_all(work_dirpath: Path) -> None: + script = work_dirpath / 'pytest_get_pytest_cases_multi_all.py' script.write_text(TEMPLATE_SCRIPT) - cases = get_pytest_cases([str(tmp_path)], CollectMode.MULTI_ALL_WITH_PARAM) + cases = get_pytest_cases([str(work_dirpath)], CollectMode.MULTI_ALL_WITH_PARAM) assert len(cases) == 2 assert cases[0].targets == ['esp32', 'esp32s2'] assert cases[1].targets == ['esp32s2', 'esp32s2', 'esp32s3'] -def test_get_pytest_cases_all(tmp_path: Path) -> None: - script = tmp_path / 'pytest_get_pytest_cases_all.py' +def test_get_pytest_cases_all(work_dirpath: Path) -> None: + script = work_dirpath / 'pytest_get_pytest_cases_all.py' script.write_text(TEMPLATE_SCRIPT) - cases = get_pytest_cases([str(tmp_path)], CollectMode.ALL) + cases = get_pytest_cases([str(work_dirpath)], CollectMode.ALL) assert len(cases) == 6 assert cases[0].targets == ['esp32', 'esp32s2'] From 142a3c8a7ae2c9ef5dc4c968dd4eb3d5f9b111f0 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Tue, 23 Jan 2024 10:32:14 +0100 Subject: [PATCH 2/3] ci: fix idf_relpath issue while searching the apps Now `PytestCase`, collect_app_info file, are all using relpath to idf instead --- conftest.py | 4 ++-- tools/ci/idf_ci_utils.py | 13 ++++++++++++ tools/ci/idf_pytest/constants.py | 14 ++++++++----- tools/ci/idf_pytest/plugin.py | 11 +++-------- tools/ci/idf_pytest/script.py | 34 +++++++++++++++++++++----------- 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/conftest.py b/conftest.py index 06699f2e6a..0d2abe9f08 100644 --- a/conftest.py +++ b/conftest.py @@ -40,7 +40,7 @@ from artifacts_handler import ArtifactType from dynamic_pipelines.constants import TEST_RELATED_APPS_DOWNLOAD_URLS_FILENAME from idf_ci.app import import_apps_from_txt from idf_ci.uploader import AppDownloader, AppUploader -from idf_ci_utils import IDF_PATH +from idf_ci_utils import IDF_PATH, idf_relpath from idf_pytest.constants import DEFAULT_SDKCONFIG, ENV_MARKERS, SPECIAL_MARKERS, TARGET_MARKERS, PytestCase from idf_pytest.plugin import IDF_PYTEST_EMBEDDED_KEY, ITEM_PYTEST_CASE_KEY, IdfPytestEmbedded from idf_pytest.utils import format_case_id @@ -219,7 +219,7 @@ def build_dir( case: PytestCase = request._pyfuncitem.stash[ITEM_PYTEST_CASE_KEY] if app_downloader: # somehow hardcoded... - app_build_path = os.path.join(os.path.relpath(app_path, IDF_PATH), f'build_{target}_{config}') + app_build_path = os.path.join(idf_relpath(app_path), f'build_{target}_{config}') if case.requires_elf_or_map: app_downloader.download_app(app_build_path) else: diff --git a/tools/ci/idf_ci_utils.py b/tools/ci/idf_ci_utils.py index ecabd22ada..d8d33d4635 100644 --- a/tools/ci/idf_ci_utils.py +++ b/tools/ci/idf_ci_utils.py @@ -248,3 +248,16 @@ def sanitize_job_name(name: str) -> str: :return: sanitized job name """ return re.sub(r' \d+/\d+', '', name) + + +def idf_relpath(p: str) -> str: + """ + Turn all paths under IDF_PATH to relative paths + :param p: path + :return: relpath to IDF_PATH, or absolute path if not under IDF_PATH + """ + abs_path = os.path.abspath(p) + if abs_path.startswith(IDF_PATH): + return os.path.relpath(abs_path, IDF_PATH) + else: + return abs_path diff --git a/tools/ci/idf_pytest/constants.py b/tools/ci/idf_pytest/constants.py index 780975bdc1..77fbb5b21f 100644 --- a/tools/ci/idf_pytest/constants.py +++ b/tools/ci/idf_pytest/constants.py @@ -12,6 +12,7 @@ from pathlib import Path from _pytest.python import Function from idf_ci_utils import IDF_PATH +from idf_ci_utils import idf_relpath from pytest_embedded.utils import to_list SUPPORTED_TARGETS = ['esp32', 'esp32s2', 'esp32c3', 'esp32s3', 'esp32c2', 'esp32c6', 'esp32h2', 'esp32p4'] @@ -149,11 +150,14 @@ class CollectMode(str, Enum): ALL = 'all' -@dataclass class PytestApp: - path: str - target: str - config: str + """ + Pytest App with relative path to IDF_PATH + """ + def __init__(self, path: str, target: str, config: str) -> None: + self.path = idf_relpath(path) + self.target = target + self.config = config def __hash__(self) -> int: return hash((self.path, self.target, self.config)) @@ -245,7 +249,7 @@ class PytestCase: if 'jtag' in self.env_markers or 'usb_serial_jtag' in self.env_markers: return True - if any('panic' in Path(app.path).resolve().parts for app in self.apps): + if any('panic' in Path(app.path).parts for app in self.apps): return True return False diff --git a/tools/ci/idf_pytest/plugin.py b/tools/ci/idf_pytest/plugin.py index 7cf1a4b774..34c4ff9aab 100644 --- a/tools/ci/idf_pytest/plugin.py +++ b/tools/ci/idf_pytest/plugin.py @@ -13,6 +13,7 @@ from _pytest.python import Function from _pytest.runner import CallInfo from idf_build_apps import App from idf_build_apps.constants import BuildStatus +from idf_ci_utils import idf_relpath from pytest_embedded import Dut from pytest_embedded.plugin import parse_multi_dut_args from pytest_embedded.utils import find_by_suffix @@ -73,7 +74,7 @@ class IdfPytestEmbedded: self._single_target_duplicate_mode = single_target_duplicate_mode self.apps_list = ( - [os.path.join(app.app_dir, app.build_dir) for app in apps if app.build_status == BuildStatus.SUCCESS] + [os.path.join(idf_relpath(app.app_dir), app.build_dir) for app in apps if app.build_status == BuildStatus.SUCCESS] if apps else None ) @@ -114,14 +115,8 @@ class IdfPytestEmbedded: configs = to_list(parse_multi_dut_args(count, self.get_param(item, 'config', DEFAULT_SDKCONFIG))) targets = to_list(parse_multi_dut_args(count, self.get_param(item, 'target', self.target[0]))) - def abspath_or_relpath(s: str) -> str: - if os.path.abspath(s) and s.startswith(os.getcwd()): - return os.path.relpath(s) - - return s - return PytestCase( - [PytestApp(abspath_or_relpath(app_paths[i]), targets[i], configs[i]) for i in range(count)], item + [PytestApp(app_paths[i], targets[i], configs[i]) for i in range(count)], item ) @pytest.hookimpl(tryfirst=True) diff --git a/tools/ci/idf_pytest/script.py b/tools/ci/idf_pytest/script.py index 7e3232c0c7..dc7a56c4ee 100644 --- a/tools/ci/idf_pytest/script.py +++ b/tools/ci/idf_pytest/script.py @@ -1,6 +1,5 @@ -# SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 - import fnmatch import io import logging @@ -11,14 +10,22 @@ from pathlib import Path import pytest from _pytest.config import ExitCode -from idf_build_apps import App, find_apps -from idf_build_apps.constants import SUPPORTED_TARGETS, BuildStatus +from idf_build_apps import App +from idf_build_apps import find_apps +from idf_build_apps.constants import BuildStatus +from idf_build_apps.constants import SUPPORTED_TARGETS from idf_ci.app import IdfCMakeApp -from idf_ci_utils import IDF_PATH, get_all_manifest_files, to_list +from idf_ci_utils import get_all_manifest_files +from idf_ci_utils import IDF_PATH +from idf_ci_utils import idf_relpath +from idf_ci_utils import to_list from idf_py_actions.constants import PREVIEW_TARGETS as TOOLS_PREVIEW_TARGETS from idf_py_actions.constants import SUPPORTED_TARGETS as TOOLS_SUPPORTED_TARGETS -from .constants import DEFAULT_BUILD_LOG_FILENAME, DEFAULT_CONFIG_RULES_STR, CollectMode, PytestCase +from .constants import CollectMode +from .constants import DEFAULT_BUILD_LOG_FILENAME +from .constants import DEFAULT_CONFIG_RULES_STR +from .constants import PytestCase from .plugin import IdfPytestEmbedded @@ -176,27 +183,30 @@ def get_all_apps( ) # app_path, target, config - pytest_app_path_tuple_dict: t.Dict[t.Tuple[Path, str, str], PytestCase] = {} + pytest_app_path_tuple_dict: t.Dict[t.Tuple[str, str, str], PytestCase] = {} for case in pytest_cases: for app in case.apps: - pytest_app_path_tuple_dict[(Path(app.path), app.target, app.config)] = case + pytest_app_path_tuple_dict[(app.path, app.target, app.config)] = case - modified_pytest_app_path_tuple_dict: t.Dict[t.Tuple[Path, str, str], PytestCase] = {} + modified_pytest_app_path_tuple_dict: t.Dict[t.Tuple[str, str, str], PytestCase] = {} for case in modified_pytest_cases: for app in case.apps: - modified_pytest_app_path_tuple_dict[(Path(app.path), app.target, app.config)] = case + modified_pytest_app_path_tuple_dict[(app.path, app.target, app.config)] = case test_related_apps: t.Set[App] = set() non_test_related_apps: t.Set[App] = set() for app in all_apps: + # PytestCase.app.path is idf_relpath + app_path = idf_relpath(app.app_dir) + # override build_status if test script got modified - if case := modified_pytest_app_path_tuple_dict.get((Path(app.app_dir), app.target, app.config_name)): + if case := modified_pytest_app_path_tuple_dict.get((app_path, app.target, app.config_name)): test_related_apps.add(app) app.build_status = BuildStatus.SHOULD_BE_BUILT app.preserve = True logging.debug('Found app: %s - required by modified test case %s', app, case.path) elif app.build_status != BuildStatus.SKIPPED: - if case := pytest_app_path_tuple_dict.get((Path(app.app_dir), app.target, app.config_name)): + if case := pytest_app_path_tuple_dict.get((app_path, app.target, app.config_name)): test_related_apps.add(app) # should be built if app.build_status = BuildStatus.SHOULD_BE_BUILT From 6895ff11b60a15accd3868a471795991e59bed1e Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Fri, 26 Jan 2024 14:09:33 +0100 Subject: [PATCH 3/3] ci: fix get_all_apps for multi targets --- tools/ci/idf_pytest/script.py | 41 +++++++++++-------- .../ci/idf_pytest/tests/test_get_all_apps.py | 15 ++++++- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/tools/ci/idf_pytest/script.py b/tools/ci/idf_pytest/script.py index dc7a56c4ee..6dabe65863 100644 --- a/tools/ci/idf_pytest/script.py +++ b/tools/ci/idf_pytest/script.py @@ -55,6 +55,8 @@ def get_pytest_cases( apps: t.Optional[t.List[App]] = None, ) -> t.List[PytestCase]: """ + Return the list of test cases + For single-dut test cases, `target` could be - [TARGET], e.g. `esp32`, to get the test cases for the given target - or `single_all`, to get all single-dut test cases @@ -143,24 +145,27 @@ def get_all_apps( :param ignore_app_dependencies_filepatterns: ignore app dependencies filepatterns :return: tuple of test-required apps and non-test-related apps """ - all_apps = find_apps( - paths, - target, - build_system=IdfCMakeApp, - recursive=True, - build_dir='build_@t_@w', - config_rules_str=config_rules_str or DEFAULT_CONFIG_RULES_STR, - build_log_filename=DEFAULT_BUILD_LOG_FILENAME, - size_json_filename='size.json', - check_warnings=True, - manifest_rootpath=IDF_PATH, - manifest_files=get_all_manifest_files(), - default_build_targets=SUPPORTED_TARGETS + (extra_default_build_targets or []), - modified_components=modified_components, - modified_files=modified_files, - ignore_app_dependencies_filepatterns=ignore_app_dependencies_filepatterns, - include_skipped_apps=True, - ) + # target could be comma separated list + all_apps: t.List[App] = [] + for _t in set(target.split(',')): + all_apps.extend(find_apps( + paths, + _t, + build_system=IdfCMakeApp, + recursive=True, + build_dir='build_@t_@w', + config_rules_str=config_rules_str or DEFAULT_CONFIG_RULES_STR, + build_log_filename=DEFAULT_BUILD_LOG_FILENAME, + size_json_filename='size.json', + check_warnings=True, + manifest_rootpath=IDF_PATH, + manifest_files=get_all_manifest_files(), + default_build_targets=SUPPORTED_TARGETS + (extra_default_build_targets or []), + modified_components=modified_components, + modified_files=modified_files, + ignore_app_dependencies_filepatterns=ignore_app_dependencies_filepatterns, + include_skipped_apps=True, + )) pytest_cases = get_pytest_cases( paths, diff --git a/tools/ci/idf_pytest/tests/test_get_all_apps.py b/tools/ci/idf_pytest/tests/test_get_all_apps.py index 6996c4d5bb..a6adab8010 100644 --- a/tools/ci/idf_pytest/tests/test_get_all_apps.py +++ b/tools/ci/idf_pytest/tests/test_get_all_apps.py @@ -75,11 +75,22 @@ def test_foo(dut): """ ) - test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='esp32s2,esp32s3') + assert len(test_related_apps) == 2 + assert len(non_test_related_apps) == 0 - assert len(test_related_apps) == 3 # 32, s2, s3 + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='esp32,esp32s3,esp32') + assert len(test_related_apps) == 2 + assert len(non_test_related_apps) == 0 + + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='all') + assert len(test_related_apps) == 3 assert len(non_test_related_apps) == len(SUPPORTED_TARGETS) - 3 + test_related_apps, non_test_related_apps = get_all_apps([str(work_dirpath)], target='foo,bar') + assert len(test_related_apps) == 0 + assert len(non_test_related_apps) == 0 + def test_get_all_apps_modified_pytest_script(work_dirpath: Path) -> None: create_project('foo', work_dirpath)