From 9fe4ceafc79431377af909fa467e35797045f50c Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Wed, 8 May 2024 11:21:10 +0200 Subject: [PATCH 1/2] fix: make idf_size.py compatible with python3.8 Previous 6caa4a17ace9 ("fix: display correct help in the idf_size.py wrapper") introduced a regression, because it uses exit_on_error parameter for argparse.ArgumentParser, which was added in python3.9, making idf_size.py incompatible with idf.py minimal required python3.8. The objective is to inspect the arguments of idf_size.py using a wrapper argparse to determine whether the legacy or refactored version should be initiated, while always displaying help for the underlying version. The exit_on_error function was previously utilized to prevent argparse from exiting and displaying help/usage. This replaces exit_on_error with a workaround that makes the --format argument optional. Since this is the sole instance where the wrapper argparse might fail, it achieves the same outcome as using exit_on_error. Fixes: 6caa4a17ace9 ("fix: display correct help in the idf_size.py wrapper") Signed-off-by: Frantisek Hrbata --- tools/idf_size.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/idf_size.py b/tools/idf_size.py index 80c968239a..a3cb75c7da 100755 --- a/tools/idf_size.py +++ b/tools/idf_size.py @@ -13,8 +13,13 @@ if __name__ == '__main__': # Here the argparse is used only to "peek" into arguments if # legacy version is requested or if old json format is specified. # In these two cases the esp_idf_size legacy version is spawned. - parser = argparse.ArgumentParser(exit_on_error=False, add_help=False) - parser.add_argument('--format') + parser = argparse.ArgumentParser(add_help=False) + # Make the --format arg optional, so this argparse instance does not + # fail with an error and the proper underlying help is displayed. + # Note that exit_on_error is supported from python3.9, so this is + # a workaround how to make sure that the args parsing doesn't fail here if idf_size.py + # is invoked e.g. without specifying the format, like "idf_size.py --format". + parser.add_argument('--format', nargs='?') parser.add_argument('-l', '--legacy', action='store_true', default=os.environ.get('ESP_IDF_SIZE_LEGACY', '0') == '1') # The sys.argv is parsed with "exit_on_error", but the argparse.ArgumentError @@ -34,7 +39,9 @@ if __name__ == '__main__': os.environ['ESP_IDF_SIZE_NG'] = '1' if not rest or '-h' in rest or '--help' in rest: print(('Note: legacy esp_idf_size version can be invoked by specifying the -l/--legacy ' - 'option or by setting the ESP_IDF_SIZE_LEGACY environment variable.')) + 'option or by setting the ESP_IDF_SIZE_LEGACY environment variable. Additionally, the ' + 'legacy version is automatically employed when the JSON format is specified for ' + 'compatibility with previous versions.')) if args.format is not None: rest = ['--format', args.format] + rest From 9df59a219301400ca97781174aabc4ef44cf79f6 Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Wed, 8 May 2024 13:38:19 +0200 Subject: [PATCH 2/2] ci: add simple test for idf_size.py python compatibility This adds a simple test that tries to run idf_size.py help and check if the process does not exit with error. This is just to make sure that idf_size.py can be used with minimum required python version. Signed-off-by: Frantisek Hrbata --- .gitlab/ci/host-test.yml | 2 ++ .gitlab/ci/rules.yml | 1 + tools/test_idf_size/pytest.ini | 12 ++++++++++++ tools/test_idf_size/test_idf_size.py | 17 +++++++++++++++++ 4 files changed, 32 insertions(+) create mode 100644 tools/test_idf_size/pytest.ini create mode 100644 tools/test_idf_size/test_idf_size.py diff --git a/.gitlab/ci/host-test.yml b/.gitlab/ci/host-test.yml index f4b3966d94..f054b8236f 100644 --- a/.gitlab/ci/host-test.yml +++ b/.gitlab/ci/host-test.yml @@ -228,6 +228,8 @@ test_tools: - pytest --noconftest test_idf_qemu.py --junitxml=${IDF_PATH}/XUNIT_IDF_PY_QEMU.xml || stat=1 - cd ${IDF_PATH}/tools/test_mkdfu - pytest --noconftest test_mkdfu.py --junitxml=${IDF_PATH}/XUNIT_MKDFU.xml || stat=1 + - cd ${IDF_PATH}/tools/test_idf_size + - pytest --noconftest test_idf_size.py --junitxml=${IDF_PATH}/XUNIT_IDF_SIZE.xml || stat=1 - cd ${IDF_PATH} - shellcheck -s sh tools/detect_python.sh || stat=1 - shellcheck -s bash tools/detect_python.sh || stat=1 diff --git a/.gitlab/ci/rules.yml b/.gitlab/ci/rules.yml index a49c5795a9..b492bdc60c 100644 --- a/.gitlab/ci/rules.yml +++ b/.gitlab/ci/rules.yml @@ -86,6 +86,7 @@ - "tools/test_idf_py/**/*" - "tools/idf_size.py" + - "tools/test_idf_size/**/*" - "tools/tools.json" - "tools/tools_schema.json" diff --git a/tools/test_idf_size/pytest.ini b/tools/test_idf_size/pytest.ini new file mode 100644 index 0000000000..d95e773e5c --- /dev/null +++ b/tools/test_idf_size/pytest.ini @@ -0,0 +1,12 @@ +[pytest] +addopts = -s -p no:pytest_embedded + +# log related +log_cli = True +log_cli_level = INFO +log_cli_format = %(asctime)s %(levelname)s %(message)s +log_cli_date_format = %Y-%m-%d %H:%M:%S + +## log all to `system-out` when case fail +junit_logging = stdout +junit_log_passing_tests = False diff --git a/tools/test_idf_size/test_idf_size.py b/tools/test_idf_size/test_idf_size.py new file mode 100644 index 0000000000..2a3baabbe0 --- /dev/null +++ b/tools/test_idf_size/test_idf_size.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python +# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import logging +import os +import sys +from pathlib import Path +from subprocess import DEVNULL +from subprocess import run + + +def test_idf_size() -> None: + # Simple test to make sure that the idf_size.py wrapper is compatible + # with idf.py minimum required python version. + logging.info('idf_size.py python compatibility check') + idf_size_path = Path(os.environ['IDF_PATH']) / 'tools' / 'idf_size.py' + run([sys.executable, idf_size_path, '--help'], stdout=DEVNULL, stderr=DEVNULL, check=True)