ci: make bringup process for check_build_test_rules.py more friendly

2022-09-06 14:58:16 +08:00 · 2022-09-06 14:58:16 +08:00 · 2d27e09b4f
commit 2d27e09b4f
--- a/.gitlab/ci/build.yml
+++ b/.gitlab/ci/build.yml
@ -211,6 +211,7 @@ build_non_test_components_apps:
  extends:
    - .build_cmake_template
    - .rules:build:component_ut
+  parallel: 2
  script:
    - set_component_ut_vars
    # CI specific options start from "--collect-size-info xxx". could ignore when running locally
--- a/.gitlab/ci/default-build-test-rules.yml
+++ b/.gitlab/ci/default-build-test-rules.yml
@ -0,0 +1,21 @@
+# this file support two keywords:
+# - extra_default_build_targets:
+#     besides of the SUPPORTED_TARGETS in IDF,
+#     enable build for the specified targets by default as well.
+# - bypass_check_test_targets:
+#     suppress the check_build_test_rules check-test-script warnings for the specified targets
+#
+# This file should ONLY be used during bringup. Should be reset to empty after the bringup process
+#
+# Take esp32c6 as an example:
+#
+#extra_default_build_targets:
+#  - esp32c6
+#
+#bypass_check_test_targets:
+#  - esp32c6
+#
+# These lines would
+# - enable the README.md check for esp32c6. Don't forget to add the build jobs in .gitlab/ci/build.yml
+# - disable the test script check with the manifest file.
+#
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@ -143,6 +143,7 @@ repos:
        language: python
        files: 'tools/test_apps/.+|examples/.+|components/.+'
        additional_dependencies:
+          - PyYAML == 5.3.1
          - idf_build_apps
      - id: sort-build-test-rules-ymls
        name: sort .build-test-rules.yml files
@ -150,6 +151,7 @@ repos:
        language: python
        files: '\.build-test-rules\.yml'
        additional_dependencies:
+          - PyYAML == 5.3.1
          - ruamel.yaml
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.0.1
--- a/tools/ci/check_build_test_rules.py
+++ b/tools/ci/check_build_test_rules.py
@ -12,6 +12,7 @@ from io import StringIO
 from pathlib import Path
 from typing import Dict, List, Optional, Tuple

+import yaml
 from idf_ci_utils import IDF_PATH, get_pytest_cases, get_ttfw_cases

 YES = u'\u2713'
@ -53,7 +54,11 @@ def doublequote(s: str) -> str:
    return f'"{s}"'


-def check_readme(paths: List[str], exclude_dirs: Optional[List[str]] = None) -> None:
+def check_readme(
+    paths: List[str],
+    exclude_dirs: Optional[List[str]] = None,
+    extra_default_build_targets: Optional[List[str]] = None,
+) -> None:
    from idf_build_apps import App, find_apps
    from idf_build_apps.constants import SUPPORTED_TARGETS

@ -142,6 +147,7 @@ def check_readme(paths: List[str], exclude_dirs: Optional[List[str]] = None) ->
            manifest_files=[
                str(p) for p in Path(IDF_PATH).glob('**/.build-test-rules.yml')
            ],
+            default_build_targets=SUPPORTED_TARGETS + extra_default_build_targets,
        )
    )
    exit_code = 0
@ -199,7 +205,11 @@ def check_readme(paths: List[str], exclude_dirs: Optional[List[str]] = None) ->
    sys.exit(exit_code)


-def check_test_scripts(paths: List[str], exclude_dirs: Optional[List[str]] = None) -> None:
+def check_test_scripts(
+    paths: List[str],
+    exclude_dirs: Optional[List[str]] = None,
+    bypass_check_test_targets: Optional[List[str]] = None,
+) -> None:
    from idf_build_apps import App, find_apps

    # takes long time, run only in CI
@ -245,7 +255,7 @@ def check_test_scripts(paths: List[str], exclude_dirs: Optional[List[str]] = Non
        actual_extra_tested_targets = set(actual_verified_targets) - set(
            _app.verified_targets
        )
-        if actual_extra_tested_targets:
+        if actual_extra_tested_targets - set(bypass_check_test_targets or []):
            print(
                inspect.cleandoc(
                    f'''
@ -401,9 +411,21 @@ if __name__ == '__main__':

    _check_readme = action.add_parser('check-readmes')
    _check_readme.add_argument('paths', nargs='+', help='check under paths')
+    _check_readme.add_argument(
+        '-c',
+        '--config',
+        default=os.path.join(IDF_PATH, '.gitlab', 'ci', 'default-build-test-rules.yml'),
+        help='default build test rules config file',
+    )

    _check_test_scripts = action.add_parser('check-test-scripts')
    _check_test_scripts.add_argument('paths', nargs='+', help='check under paths')
+    _check_test_scripts.add_argument(
+        '-c',
+        '--config',
+        default=os.path.join(IDF_PATH, '.gitlab', 'ci', 'default-build-test-rules.yml'),
+        help='default build test rules config file',
+    )

    _sort_yaml = action.add_parser('sort-yaml')
    _sort_yaml.add_argument('files', nargs='+', help='all specified yaml files')
@ -411,7 +433,9 @@ if __name__ == '__main__':
    arg = parser.parse_args()

    # Since this script is executed from the pre-commit hook environment, make sure IDF_PATH is set
-    os.environ['IDF_PATH'] = os.path.realpath(os.path.join(os.path.dirname(__file__), '..', '..'))
+    os.environ['IDF_PATH'] = os.path.realpath(
+        os.path.join(os.path.dirname(__file__), '..', '..')
+    )

    if arg.action == 'sort-yaml':
        sort_yaml(arg.files)
@ -420,7 +444,9 @@ if __name__ == '__main__':

        # check if *_caps.h files changed
        check_all = False
-        soc_caps_header_files = list((Path(IDF_PATH) / 'components' / 'soc').glob('**/*_caps.h'))
+        soc_caps_header_files = list(
+            (Path(IDF_PATH) / 'components' / 'soc').glob('**/*_caps.h')
+        )
        for p in arg.paths:
            if Path(p).resolve() in soc_caps_header_files:
                check_all = True
@ -437,7 +463,29 @@ if __name__ == '__main__':
        else:
            _exclude_dirs = []

+        extra_default_build_targets: List[str] = []
+        bypass_check_test_targets: List[str] = []
+        if arg.config:
+            with open(arg.config) as fr:
+                configs = yaml.safe_load(fr)
+
+            if configs:
+                extra_default_build_targets = (
+                    configs.get('extra_default_build_targets') or []
+                )
+                bypass_check_test_targets = (
+                    configs.get('bypass_check_test_targets') or []
+                )
+
        if arg.action == 'check-readmes':
-            check_readme(list(check_dirs), _exclude_dirs)
+            check_readme(
+                list(check_dirs),
+                exclude_dirs=_exclude_dirs,
+                extra_default_build_targets=extra_default_build_targets,
+            )
        elif arg.action == 'check-test-scripts':
-            check_test_scripts(list(check_dirs), _exclude_dirs)
+            check_test_scripts(
+                list(check_dirs),
+                exclude_dirs=_exclude_dirs,
+                bypass_check_test_targets=bypass_check_test_targets,
+            )
--- a/tools/ci/ci_build_apps.py
+++ b/tools/ci/ci_build_apps.py
@ -10,10 +10,12 @@ import os
 import sys
 from collections import defaultdict
 from pathlib import Path
-from typing import List, Set
+from typing import List, Optional, Set

+import yaml
 from idf_build_apps import LOGGER, App, build_apps, find_apps, setup_logging
-from idf_ci_utils import IDF_PATH, get_pytest_app_paths, get_pytest_cases, get_ttfw_app_paths
+from idf_build_apps.constants import SUPPORTED_TARGETS
+from idf_ci_utils import IDF_PATH, PytestApp, get_pytest_cases, get_ttfw_app_paths


 def get_pytest_apps(
@ -22,6 +24,7 @@ def get_pytest_apps(
    config_rules_str: List[str],
    marker_expr: str,
    preserve_all: bool = False,
+    extra_default_build_targets: Optional[List[str]] = None,
 ) -> List[App]:
    pytest_cases = get_pytest_cases(paths, target, marker_expr)

@ -55,9 +58,8 @@ def get_pytest_apps(
        build_log_path='build_log.txt',
        size_json_path='size.json',
        check_warnings=True,
-        manifest_files=[
-            str(p) for p in Path(IDF_PATH).glob('**/.build-test-rules.yml')
-        ],
+        manifest_files=[str(p) for p in Path(IDF_PATH).glob('**/.build-test-rules.yml')],
+        default_build_targets=SUPPORTED_TARGETS + extra_default_build_targets,
    )

    for app in apps:
@ -73,6 +75,7 @@ def get_cmake_apps(
    target: str,
    config_rules_str: List[str],
    preserve_all: bool = False,
+    extra_default_build_targets: Optional[List[str]] = None,
 ) -> List[App]:
    ttfw_app_dirs = get_ttfw_app_paths(paths, target)
    apps = find_apps(
@ -85,18 +88,17 @@ def get_cmake_apps(
        size_json_path='size.json',
        check_warnings=True,
        preserve=False,
-        manifest_files=[
-            str(p) for p in Path(IDF_PATH).glob('**/.build-test-rules.yml')
-        ],
+        manifest_files=[str(p) for p in Path(IDF_PATH).glob('**/.build-test-rules.yml')],
+        default_build_targets=SUPPORTED_TARGETS + extra_default_build_targets,
    )

    apps_for_build = []
-    pytest_app_dirs = get_pytest_app_paths(paths, target)
+    pytest_cases_apps = [app for case in get_pytest_cases(paths, target) for app in case.apps]
    for app in apps:
        if preserve_all or app.app_dir in ttfw_app_dirs:  # relpath
            app.preserve = True

-        if os.path.realpath(app.app_dir) in pytest_app_dirs:
+        if PytestApp(os.path.realpath(app.app_dir), app.target, app.config_name) in pytest_cases_apps:
            LOGGER.debug('Skipping build app with pytest scripts: %s', app)
            continue

@ -109,14 +111,33 @@ APPS_BUILD_PER_JOB = 30


 def main(args: argparse.Namespace) -> None:
+    extra_default_build_targets: List[str] = []
+    if args.default_build_test_rules:
+        with open(args.default_build_test_rules) as fr:
+            configs = yaml.safe_load(fr)
+
+        if configs:
+            extra_default_build_targets = configs.get('extra_default_build_targets') or []
+
    if args.pytest_apps:
        LOGGER.info('Only build apps with pytest scripts')
        apps = get_pytest_apps(
-            args.paths, args.target, args.config, args.marker_expr, args.preserve_all
+            args.paths,
+            args.target,
+            args.config,
+            args.marker_expr,
+            args.preserve_all,
+            extra_default_build_targets,
        )
    else:
        LOGGER.info('build apps. will skip pytest apps with pytest scripts')
-        apps = get_cmake_apps(args.paths, args.target, args.config, args.preserve_all)
+        apps = get_cmake_apps(
+            args.paths,
+            args.target,
+            args.config,
+            args.preserve_all,
+            extra_default_build_targets,
+        )

    LOGGER.info('Found %d apps after filtering', len(apps))
    LOGGER.info(
@ -131,10 +152,7 @@ def main(args: argparse.Namespace) -> None:
            for extra_preserve_dir in args.extra_preserve_dirs:
                abs_extra_preserve_dir = Path(extra_preserve_dir).resolve()
                abs_app_dir = Path(app.app_dir).resolve()
-                if (
-                    abs_extra_preserve_dir == abs_app_dir
-                    or abs_extra_preserve_dir in abs_app_dir.parents
-                ):
+                if abs_extra_preserve_dir == abs_app_dir or abs_extra_preserve_dir in abs_app_dir.parents:
                    app.preserve = True

    ret_code = build_apps(
@ -193,9 +211,7 @@ if __name__ == '__main__':
        action='store_true',
        help='Preserve the binaries for all apps when specified.',
    )
-    parser.add_argument(
-        '--parallel-count', default=1, type=int, help='Number of parallel build jobs.'
-    )
+    parser.add_argument('--parallel-count', default=1, type=int, help='Number of parallel build jobs.')
    parser.add_argument(
        '--parallel-index',
        default=1,
@ -247,6 +263,11 @@ if __name__ == '__main__':
        help='only build tests matching given mark expression. For example: -m "host_test and generic". Works only'
        'for pytest',
    )
+    parser.add_argument(
+        '--default-build-test-rules',
+        default=os.path.join(IDF_PATH, '.gitlab', 'ci', 'default-build-test-rules.yml'),
+        help='default build test rules config file',
+    )

    arguments = parser.parse_args()

--- a/tools/ci/idf_ci_utils.py
+++ b/tools/ci/idf_ci_utils.py
@ -274,14 +274,6 @@ def get_pytest_cases(
    return cases


-def get_pytest_app_paths(
-    paths: Union[str, List[str]], target: str, marker_expr: Optional[str] = None
-) -> Set[str]:
-    cases = get_pytest_cases(paths, target, marker_expr)
-
-    return set({app.path for case in cases for app in case.apps})
-
-
 ##################
 # TTFW Utilities #
 ##################