idf.py: Add support for action specific options

Changes argument parsing mechanism from argparse to a new one, that provides better support for extensions and options that are only applicable to specific subcommands,

Breaking changes:

1. All global options should go before subcommands, i.e. `idf.py build -C ~/some/project` will not work anymore, only `idf.py -C ~/some/project build` is acceptable
2. To provide multiple values to an option like `--define-cache-entry` it's necessary to repeat option many times, i.e. `idf.py -D entry1 entry2 entry3` will not work, right way is: `idf.py -D entry1 -D entry2 -D entry3`
At the moment there are 3 options like this:  `--define-cache-entry` in base list and `--test-components` and `--test-exclude-components` in the unit test extensions
3. Drops `defconfig` and `bootloader-clean` subcommands

Closes https://github.com/espressif/esp-idf/issues/3570
Closes https://github.com/espressif/esp-idf/issues/3571

examples/system/unit_test/test/CMakeLists.txt

@@ -1,4 +1,4 @@
-# This is the project CMakeLists.txt file for the test subproject 
+# This is the project CMakeLists.txt file for the test subproject
 cmake_minimum_required(VERSION 3.5)
 
 # Include the components directory of the main application:
@@ -8,7 +8,7 @@ set(EXTRA_COMPONENT_DIRS "../components")
 # Set the components to include the tests for.
 # This can be overriden from CMake cache:
 # - when invoking CMake directly: cmake -D TEST_COMPONENTS="xxxxx" ..
-# - when using idf.py: idf.py build -T xxxxx
+# - when using idf.py: idf.py -T xxxxx build
 #
 set(TEST_COMPONENTS "testable" CACHE STRING "List of components to test")
 

requirements.txt

@@ -5,6 +5,7 @@ setuptools
 # The setuptools package is required to install source distributions and on some systems is not installed by default.
 # Please keep it as the first item of this list.
 #
+click>=5.0
 pyserial>=3.0
 future>=0.15.2
 cryptography>=2.1.4

tools/ci/test_build_system_cmake.sh

@@ -348,7 +348,7 @@ function run_tests()
     echo "CONFIG_ESP32_SPIRAM_SUPPORT=y" >> sdkconfig.defaults
     echo "CONFIG_SPIRAM_CACHE_WORKAROUND=y" >> sdkconfig.defaults
     # note: we do 'reconfigure' here, as we just need to run cmake
-    idf.py -C $IDF_PATH/examples/build_system/cmake/import_lib -B `pwd`/build reconfigure -D SDKCONFIG_DEFAULTS="`pwd`/sdkconfig.defaults"
+    idf.py -C $IDF_PATH/examples/build_system/cmake/import_lib -B `pwd`/build -D SDKCONFIG_DEFAULTS="`pwd`/sdkconfig.defaults" reconfigure
     grep -q '"command"' build/compile_commands.json || failure "compile_commands.json missing or has no no 'commands' in it"
     (grep '"command"' build/compile_commands.json | grep -v mfix-esp32-psram-cache-issue) && failure "All commands in compile_commands.json should use PSRAM cache workaround"
     rm -r sdkconfig.defaults build
@@ -411,7 +411,7 @@ endmenu\n" >> ${IDF_PATH}/Kconfig;
     print_status "Check ccache is used to build when present"
     touch ccache && chmod +x ccache  # make sure that ccache is present for this test
     (export PATH=$PWD:$PATH && idf.py reconfigure | grep "ccache will be used for faster builds") || failure "ccache should be used when present"
-    (export PATH=$PWD:$PATH && idf.py reconfigure --no-ccache | grep -c "ccache will be used for faster builds" | grep -wq 0) \
+    (export PATH=$PWD:$PATH && idf.py  --no-ccache reconfigure| grep -c "ccache will be used for faster builds" | grep -wq 0) \
         || failure "ccache should not be used even when present if --no-ccache is specified"
     rm -f ccache
 

tools/unit-test-app/README.md

@@ -20,9 +20,9 @@ ESP-IDF unit tests are run using Unit Test App. The app can be built with the un
 * Set IDF_PATH environment variable to point to the path to the esp-idf top-level directory.
 * Change into `tools/unit-test-app` directory
 * `idf.py menuconfig` to configure the Unit Test App.
-* `idf.py build -T <component> <component> ...` with `component` set to names of the components to be included in the test app. Or `idf.py build -T all` to build the test app with all the tests for components having `test` subdirectory.
-* Follow the printed instructions to flash, or run `idf.py flash -p PORT`.
-* Unit test have a few preset sdkconfigs. It provides command `idf.py ut-clean-config_name` and `idf.py ut-build-config_name` (where `config_name` is the file name under `unit-test-app/configs` folder) to build with preset configs. For example, you can use `idf.py ut-build-default -T all` to build with config file `unit-test-app/configs/default`. Built binary for this config will be copied to `unit-test-app/output/config_name` folder.
+* `idf.py -T <component> -T <component> ... build` with `component` set to names of the components to be included in the test app. Or `idf.py -T all build` to build the test app with all the tests for components having `test` subdirectory.
+* Follow the printed instructions to flash, or run `idf.py -p PORT flash`.
+* Unit test have a few preset sdkconfigs. It provides command `idf.py ut-clean-config_name` and `idf.py ut-build-config_name` (where `config_name` is the file name under `unit-test-app/configs` folder) to build with preset configs. For example, you can use `idf.py -T all ut-build-default` to build with config file `unit-test-app/configs/default`. Built binary for this config will be copied to `unit-test-app/output/config_name` folder.
 
 # Flash Size
 

tools/unit-test-app/idf_ext.py

@@ -1,96 +1,40 @@
+import copy
 import glob
-import tempfile
 import os
 import os.path
 import re
 import shutil
-import argparse
-import copy
-
-PROJECT_NAME = "unit-test-app"
-PROJECT_PATH = os.getcwd()
-
-# List of unit-test-app configurations.
-# Each file in configs/ directory defines a configuration. The format is the
-# same as sdkconfig file. Configuration is applied on top of sdkconfig.defaults
-# file from the project directory
-CONFIG_NAMES = os.listdir(os.path.join(PROJECT_PATH, "configs"))
-
-# Build (intermediate) and output (artifact) directories
-BUILDS_DIR = os.path.join(PROJECT_PATH, "builds")
-BINARIES_DIR = os.path.join(PROJECT_PATH, "output")
-
-
-# Convert the values passed to the -T parameter to corresponding cache entry definitions
-# TESTS_ALL and TEST_COMPONENTS
-class TestComponentAction(argparse.Action):
-    def __call__(self, parser, namespace, values, option_string=None):
-        # Create a new of cache definition entry, adding previous elements
-        cache_entries = list()
-
-        existing_entries = getattr(namespace, "define_cache_entry", [])
-
-        if existing_entries:
-            cache_entries.extend(existing_entries)
-
-        # Form -D arguments
-        if "all" in values:
-            cache_entries.append("TESTS_ALL=1")
-            cache_entries.append("TEST_COMPONENTS=''")
-        else:
-            cache_entries.append("TESTS_ALL=0")
-            cache_entries.append("TEST_COMPONENTS='%s'" % " ".join(values))
-
-        setattr(namespace, "define_cache_entry", cache_entries)
-
-        # Brute force add reconfigure at the very beginning
-        existing_actions = getattr(namespace, "actions", [])
-        if "reconfigure" not in existing_actions:
-            existing_actions = ["reconfigure"] + existing_actions
-        setattr(namespace, "actions", existing_actions)
-
-
-class TestExcludeComponentAction(argparse.Action):
-    def __call__(self, parser, namespace, values, option_string=None):
-        # Create a new of cache definition entry, adding previous elements
-        cache_entries = list()
-
-        existing_entries = getattr(namespace, "define_cache_entry", [])
-
-        if existing_entries:
-            cache_entries.extend(existing_entries)
-
-        cache_entries.append("TEST_EXCLUDE_COMPONENTS='%s'" % " ".join(values))
-
-        setattr(namespace, "define_cache_entry", cache_entries)
+import tempfile
 
-        # Brute force add reconfigure at the very beginning
-        existing_actions = getattr(namespace, "actions", [])
-        if "reconfigure" not in existing_actions:
-            existing_actions = ["reconfigure"] + existing_actions
-        setattr(namespace, "actions", existing_actions)
 
+def action_extensions(base_actions, project_path=os.getcwd()):
+    """ Describes extensions for unit tests. This function expects that actions "all" and "reconfigure" """
 
-def add_argument_extensions(parser):
-    # For convenience, define a -T argument that gets converted to -D arguments
-    parser.add_argument('-T', '--test-component', help="Specify the components to test", nargs='+', action=TestComponentAction)
-    # For convenience, define a -T argument that gets converted to -D arguments
-    parser.add_argument('-E', '--test-exclude-components', help="Specify the components to exclude from testing", nargs='+', action=TestExcludeComponentAction)
+    PROJECT_NAME = "unit-test-app"
 
+    # List of unit-test-app configurations.
+    # Each file in configs/ directory defines a configuration. The format is the
+    # same as sdkconfig file. Configuration is applied on top of sdkconfig.defaults
+    # file from the project directory
+    CONFIG_NAMES = os.listdir(os.path.join(project_path, "configs"))
 
-def add_action_extensions(base_functions, base_actions):
+    # Build (intermediate) and output (artifact) directories
+    BUILDS_DIR = os.path.join(project_path, "builds")
+    BINARIES_DIR = os.path.join(project_path, "output")
 
-    def ut_apply_config(ut_apply_config_name, args):
+    def ut_apply_config(ut_apply_config_name, ctx, args):
         config_name = re.match(r"ut-apply-config-(.*)", ut_apply_config_name).group(1)
 
         def set_config_build_variables(prop, defval=None):
-            property_value = re.findall(r"^%s=(.+)" % prop, config_file_content, re.MULTILINE)
-            if (property_value):
+            property_value = re.findall(
+                r"^%s=(.+)" % prop, config_file_content, re.MULTILINE
+            )
+            if property_value:
                 property_value = property_value[0]
             else:
                 property_value = defval
 
-            if (property_value):
+            if property_value:
                 try:
                     args.define_cache_entry.append("%s=" % prop + property_value)
                 except AttributeError:
@@ -116,7 +60,7 @@ def add_action_extensions(base_functions, base_actions):
 
         if config_name in CONFIG_NAMES:
             # Parse the sdkconfig for components to be included/excluded and tests to be run
-            config = os.path.join(PROJECT_PATH, "configs", config_name)
+            config = os.path.join(project_path, "configs", config_name)
 
             with open(config, "r") as config_file:
                 config_file_content = config_file.read()
@@ -136,17 +80,17 @@ def add_action_extensions(base_functions, base_actions):
                 except AttributeError:
                     args.define_cache_entry = [tests_all]
 
-                set_config_build_variables("TEST_EXCLUDE_COMPONENTS","''")
+                set_config_build_variables("TEST_EXCLUDE_COMPONENTS", "''")
 
             with tempfile.NamedTemporaryFile() as sdkconfig_temp:
                 # Use values from the combined defaults and the values from
                 # config folder to build config
-                sdkconfig_default = os.path.join(PROJECT_PATH, "sdkconfig.defaults")
+                sdkconfig_default = os.path.join(project_path, "sdkconfig.defaults")
 
                 with open(sdkconfig_default, "rb") as sdkconfig_default_file:
                     sdkconfig_temp.write(sdkconfig_default_file.read())
 
-                sdkconfig_config = os.path.join(PROJECT_PATH, "configs", config_name)
+                sdkconfig_config = os.path.join(project_path, "configs", config_name)
                 with open(sdkconfig_config, "rb") as sdkconfig_config_file:
                     sdkconfig_temp.write(b"\n")
                     sdkconfig_temp.write(sdkconfig_config_file.read())
@@ -154,21 +98,28 @@ def add_action_extensions(base_functions, base_actions):
                 sdkconfig_temp.flush()
 
                 try:
-                    args.define_cache_entry.append("SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name)
+                    args.define_cache_entry.append(
+                        "SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name
+                    )
                 except AttributeError:
-                    args.define_cache_entry = ["SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name]
+                    args.define_cache_entry = [
+                        "SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name
+                    ]
 
-                reconfigure = base_functions["reconfigure"]
-                reconfigure(None, args)
+                reconfigure = base_actions["actions"]["reconfigure"]["callback"]
+                reconfigure(None, ctx, args)
         else:
             if not config_name == "all-configs":
-                print("unknown unit test app config for action '%s'" % ut_apply_config_name)
+                print(
+                    "unknown unit test app config for action '%s'"
+                    % ut_apply_config_name
+                )
 
     # This target builds the configuration. It does not currently track dependencies,
     # but is good enough for CI builds if used together with clean-all-configs.
     # For local builds, use 'apply-config-NAME' target and then use normal 'all'
     # and 'flash' targets.
-    def ut_build(ut_build_name, args):
+    def ut_build(ut_build_name, ctx, args):
         # Create a copy of the passed arguments to prevent arg modifications to accrue if
         # all configs are being built
         build_args = copy.copy(args)
@@ -187,14 +138,17 @@ def add_action_extensions(base_functions, base_actions):
                 pass
 
             # Build, tweaking paths to sdkconfig and sdkconfig.defaults
-            ut_apply_config("ut-apply-config-" + config_name, build_args)
+            ut_apply_config("ut-apply-config-" + config_name, ctx, build_args)
 
-            build_target = base_functions["build_target"]
+            build_target = base_actions["actions"]["all"]["callback"]
 
-            build_target("all", build_args)
+            build_target("all", ctx, build_args)
 
             # Copy artifacts to the output directory
-            shutil.copyfile(os.path.join(build_args.project_dir, "sdkconfig"), os.path.join(dest, "sdkconfig"))
+            shutil.copyfile(
+                os.path.join(build_args.project_dir, "sdkconfig"),
+                os.path.join(dest, "sdkconfig"),
+            )
 
             binaries = [PROJECT_NAME + x for x in [".elf", ".bin", ".map"]]
 
@@ -206,16 +160,29 @@ def add_action_extensions(base_functions, base_actions):
             except OSError:
                 pass
 
-            shutil.copyfile(os.path.join(src, "bootloader", "bootloader.bin"), os.path.join(dest, "bootloader", "bootloader.bin"))
+            shutil.copyfile(
+                os.path.join(src, "bootloader", "bootloader.bin"),
+                os.path.join(dest, "bootloader", "bootloader.bin"),
+            )
 
-            for partition_table in glob.glob(os.path.join(src, "partition_table", "partition-table*.bin")):
+            for partition_table in glob.glob(
+                os.path.join(src, "partition_table", "partition-table*.bin")
+            ):
                 try:
                     os.mkdir(os.path.join(dest, "partition_table"))
                 except OSError:
                     pass
-                shutil.copyfile(partition_table, os.path.join(dest, "partition_table", os.path.basename(partition_table)))
-
-            shutil.copyfile(os.path.join(src, "flasher_args.json"), os.path.join(dest, "flasher_args.json"))
+                shutil.copyfile(
+                    partition_table,
+                    os.path.join(
+                        dest, "partition_table", os.path.basename(partition_table)
+                    ),
+                )
+
+            shutil.copyfile(
+                os.path.join(src, "flasher_args.json"),
+                os.path.join(dest, "flasher_args.json"),
+            )
 
             binaries = glob.glob(os.path.join(src, "*.bin"))
             binaries = [os.path.basename(s) for s in binaries]
@@ -227,7 +194,7 @@ def add_action_extensions(base_functions, base_actions):
             if not config_name == "all-configs":
                 print("unknown unit test app config for action '%s'" % ut_build_name)
 
-    def ut_clean(ut_clean_name, args):
+    def ut_clean(ut_clean_name, ctx, args):
         config_name = re.match(r"ut-clean-(.*)", ut_clean_name).group(1)
         if config_name in CONFIG_NAMES:
             shutil.rmtree(os.path.join(BUILDS_DIR, config_name), ignore_errors=True)
@@ -236,26 +203,52 @@ def add_action_extensions(base_functions, base_actions):
             if not config_name == "all-configs":
                 print("unknown unit test app config for action '%s'" % ut_clean_name)
 
-    def ut_help(action, args):
-        HELP_STRING = """
-Additional unit-test-app specific targets
-
-idf.py ut-build-NAME - Build unit-test-app with configuration provided in configs/NAME.
-                    Build directory will be builds/NAME/, output binaries will be
-                    under output/NAME/
+    def test_component_callback(ctx, global_args, tasks):
+        """ Convert the values passed to the -T and -E parameter to corresponding cache entry definitions TESTS_ALL and TEST_COMPONENTS """
+        test_components = global_args.test_components
+        test_exclude_components = global_args.test_exclude_components
 
-idf.py ut-clean-NAME - Remove build and output directories for configuration NAME.
+        cache_entries = []
 
-idf.py ut-build-all-configs - Build all configurations defined in configs/ directory.
-
-idf.py ut-apply-config-NAME - Generates configuration based on configs/NAME in sdkconfig
-                    file. After this, normal all/flash targets can be used.
-                    Useful for development/debugging.
-"""
-        print(HELP_STRING)
-
-    # Build dictionary of action extensions
-    extensions = dict()
+        if test_components:
+            if "all" in test_components:
+                cache_entries.append("TESTS_ALL=1")
+                cache_entries.append("TEST_COMPONENTS=''")
+            else:
+                cache_entries.append("TESTS_ALL=0")
+                cache_entries.append("TEST_COMPONENTS='%s'" % " ".join(test_components))
+
+        if test_exclude_components:
+            cache_entries.append(
+                "TEST_EXCLUDE_COMPONENTS='%s'" % " ".join(test_exclude_components)
+            )
+
+        if cache_entries:
+            global_args.define_cache_entry = list(global_args.define_cache_entry)
+            global_args.define_cache_entry.extend(cache_entries)
+
+            # Brute force add reconfigure at the very beginning
+            reconfigure_task = ctx.invoke(ctx.command.get_command(ctx, "reconfigure"))
+            tasks.insert(0, reconfigure_task)
+
+    # Add global options
+    extensions = {
+        "global_options": [
+            # For convenience, define a -T and -E argument that gets converted to -D arguments
+            {
+                "names": ["-T", "--test-components"],
+                "help": "Specify the components to test",
+                "multiple": True,
+            },
+            {
+                "names": ["-E", "--test-exclude-components"],
+                "help": "Specify the components to exclude from testing",
+                "multiple": True,
+            },
+        ],
+        "global_action_callbacks": [test_component_callback],
+        "actions": {},
+    }
 
     # This generates per-config targets (clean, build, apply-config).
     build_all_config_deps = []
@@ -266,16 +259,39 @@ idf.py ut-apply-config-NAME - Generates configuration based on configs/NAME in s
         config_clean_action_name = "ut-clean-" + config
         config_apply_config_action_name = "ut-apply-config-" + config
 
-        extensions[config_build_action_name] = (ut_build, [], [])
-        extensions[config_clean_action_name] = (ut_clean, [], [])
-        extensions[config_apply_config_action_name] = (ut_apply_config, [], [])
+        extensions["actions"][config_build_action_name] = {
+            "callback": ut_build,
+            "help": "Build unit-test-app with configuration provided in configs/NAME. "
+            + "Build directory will be builds/%s/, " % config_build_action_name
+            + "output binaries will be under output/%s/" % config_build_action_name,
+        }
+
+        extensions["actions"][config_clean_action_name] = {
+            "callback": ut_clean,
+            "help": "Remove build and output directories for configuration %s."
+            % config_clean_action_name,
+        }
+
+        extensions["actions"][config_apply_config_action_name] = {
+            "callback": ut_apply_config,
+            "help": "Generates configuration based on configs/%s in sdkconfig file."
+            % config_apply_config_action_name
+            + "After this, normal all/flash targets can be used. Useful for development/debugging.",
+        }
 
         build_all_config_deps.append(config_build_action_name)
         clean_all_config_deps.append(config_clean_action_name)
 
-    extensions["ut-build-all-configs"] = (ut_build, build_all_config_deps, [])
-    extensions["ut-clean-all-configs"] = (ut_clean, clean_all_config_deps, [])
+    extensions["actions"]["ut-build-all-configs"] = {
+        "callback": ut_build,
+        "help": "Build all configurations defined in configs/ directory.",
+        "dependencies": build_all_config_deps,
+    }
 
-    extensions["ut-help"] = (ut_help, [], [])
+    extensions["actions"]["ut-clean-all-configs"] = {
+        "callback": ut_clean,
+        "help": "Remove build and output directories for all configurations defined in configs/ directory.",
+        "dependencies": clean_all_config_deps,
+    }
 
-    base_actions.update(extensions)
+    return extensions