From 13642625800e10f1ecc04912547cef6794c03c1a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Sat, 21 Jan 2023 14:48:22 -0500 Subject: [PATCH] Zap regen all golden images (#24527) * Regenerate golden images during zap regen all * Restyle * Ensure output is in out. For some reason using tmp directory does not make the diff work. Need to debug that one ... * Add a comment explaining why the temporary directory is in the out directory * Explain why temporary directory in out is needed * Make test_generate try to use python3 if available * Added comments on various code points based on review --- scripts/tools/zap/test_generate.py | 2 +- scripts/tools/zap_regen_all.py | 174 ++++++++++++++++++++++------- 2 files changed, 135 insertions(+), 41 deletions(-) diff --git a/scripts/tools/zap/test_generate.py b/scripts/tools/zap/test_generate.py index ce18acc2fca397..f6ea2ab3d5abfb 100755 --- a/scripts/tools/zap/test_generate.py +++ b/scripts/tools/zap/test_generate.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # Copyright (c) 2022 Project CHIP Authors # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/scripts/tools/zap_regen_all.py b/scripts/tools/zap_regen_all.py index 8e2e4a9a52e94e..9b2a78f3489f1d 100755 --- a/scripts/tools/zap_regen_all.py +++ b/scripts/tools/zap_regen_all.py @@ -19,16 +19,47 @@ import logging import multiprocessing import os +import shutil import subprocess import sys +import tempfile import time from dataclasses import dataclass +from enum import Flag, auto from pathlib import Path CHIP_ROOT_DIR = os.path.realpath( os.path.join(os.path.dirname(__file__), '../..')) +# Type of targets that can be re-generated +class TargetType(Flag): + + # Tests for golden images + TESTS = auto() + + # Global templates: generally examples and chip controller + GLOBAL = auto() + + # App-specific templates (see getSpecificTemplatesTargets) + SPECIFIC = auto() + + # Golden compares for unit tests of zap codegen + GOLDEN_TEST_IMAGES = auto() + + # All possible targets. Convenience constant + ALL = TESTS | GLOBAL | SPECIFIC | GOLDEN_TEST_IMAGES + + +__TARGET_TYPES__ = { + 'tests': TargetType.TESTS, + 'global': TargetType.GLOBAL, + 'specific': TargetType.SPECIFIC, + 'golden_test_images': TargetType.GOLDEN_TEST_IMAGES, + 'all': TargetType.ALL, +} + + @dataclass class TargetRunStats: config: str @@ -113,6 +144,45 @@ def generate(self) -> TargetRunStats: ) +class GoldenTestImageTarget(): + def __init__(self): + # NOTE: output-path is inside the tree. This is because clang-format + # will search for a .clang-format file in the directory tree + # so attempts to format outside the tree will generate diffs. + # NOTE: relative path because this script generally does a + # os.chdir to CHIP_ROOT anyway. + os.makedirs('./out', exist_ok=True) + self.tempdir = tempfile.mkdtemp(prefix='test_golden', dir='./out') + + # This runs a test, but the important bit is we pass `--regenerate` + # to it and this will cause it to OVERWRITE golden images. + self.command = ["./scripts/tools/zap/test_generate.py", + "--output", self.tempdir, "--regenerate"] + + def __del__(self): + # Clean up + if os.path.isdir(self.tempdir): + shutil.rmtree(self.tempdir) + + def generate(self) -> TargetRunStats: + generate_start = time.time() + subprocess.check_call(self.command) + generate_end = time.time() + + return TargetRunStats( + generate_time=generate_end - generate_start, + config='./scripts/tools/zap/test_generate.py', + template='./scripts/tools/zap/test_generate.py', + ) + + def distinct_output(self): + # Fake output - this is a single target that generates golden images + return ZapDistinctOutput(input_template='GOLDEN_IMAGES', output_directory='GOLDEN_IMAGES') + + def log_command(self): + logging.info(" %s" % " ".join(self.command)) + + def checkPythonVersion(): if sys.version_info[0] < 3: print('Must use Python 3. Current version is ' + @@ -123,7 +193,7 @@ def checkPythonVersion(): def setupArgumentsParser(): parser = argparse.ArgumentParser( description='Generate content from ZAP files') - parser.add_argument('--type', default='all', choices=['all', 'tests'], + parser.add_argument('--type', action='append', choices=__TARGET_TYPES__.keys(), help='Choose which content type to generate (default: all)') parser.add_argument('--tests', default='all', choices=['all', 'chip-tool', 'darwin-framework-tool', 'app1', 'app2'], help='When generating tests only target, Choose which tests to generate (default: all)') @@ -136,7 +206,21 @@ def setupArgumentsParser(): parser.add_argument('--no-parallel', action='store_false', dest='parallel') parser.set_defaults(parallel=True) - return parser.parse_args() + args = parser.parse_args() + + # Convert a list of target_types (as strings) + # into a single flag value + if not args.type: + args.type = TargetType.ALL # default instead of a list + else: + # convert the list into a single flag value + types = [t for t in map(lambda x: __TARGET_TYPES__[ + x.lower()], args.type)] + args.type = types[0] + for t in types: + args.type = args.type | t + + return args def getGlobalTemplatesTargets(): @@ -233,6 +317,10 @@ def getTestsTemplatesTargets(test_target): return targets +def getGoldenTestImageTargets(): + return [GoldenTestImageTarget()] + + def getSpecificTemplatesTargets(): zap_filepath = 'src/controller/data_model/controller-clusters.zap' @@ -259,12 +347,17 @@ def getSpecificTemplatesTargets(): def getTargets(type, test_target): targets = [] - if type == 'all': - targets.extend(getGlobalTemplatesTargets()) + if type & TargetType.TESTS: targets.extend(getTestsTemplatesTargets('all')) + + if type & TargetType.GLOBAL: + targets.extend(getGlobalTemplatesTargets()) + + if type & TargetType.SPECIFIC: targets.extend(getSpecificTemplatesTargets()) - elif type == 'tests': - targets.extend(getTestsTemplatesTargets(test_target)) + + if type & TargetType.GOLDEN_TEST_IMAGES: + targets.extend(getGoldenTestImageTargets()) logging.info("Targets to be generated:") for target in targets: @@ -308,40 +401,41 @@ def main(): targets = getTargets(args.type, args.tests) - if not args.dry_run: - - if args.run_bootstrap: - subprocess.check_call(os.path.join( - CHIP_ROOT_DIR, "scripts/tools/zap/zap_bootstrap.sh"), shell=True) - - timings = [] - if args.parallel: - # Ensure each zap run is independent - os.environ['ZAP_TEMPSTATE'] = '1' - with multiprocessing.Pool() as pool: - for timing in pool.imap_unordered(_ParallelGenerateOne, targets): - timings.append(timing) - else: - for target in targets: - timings.append(target.generate()) - - timings.sort(key=lambda t: t.generate_time) - - print(" Time (s) | {:^50} | {:^50}".format("Config", "Template")) - for timing in timings: - tmpl = timing.template - - if len(tmpl) > 50: - # easier to distinguish paths ... shorten common in-fixes - tmpl = tmpl.replace("/zap-templates/", "/../") - tmpl = tmpl.replace("/templates/", "/../") - - print(" %8d | %50s | %50s" % ( - timing.generate_time, - ".." + timing.config[len(timing.config) - - 48:] if len(timing.config) > 50 else timing.config, - ".." + tmpl[len(tmpl) - 48:] if len(tmpl) > 50 else tmpl, - )) + if args.dry_run: + sys.exit(0) + + if args.run_bootstrap: + subprocess.check_call(os.path.join( + CHIP_ROOT_DIR, "scripts/tools/zap/zap_bootstrap.sh"), shell=True) + + timings = [] + if args.parallel: + # Ensure each zap run is independent + os.environ['ZAP_TEMPSTATE'] = '1' + with multiprocessing.Pool() as pool: + for timing in pool.imap_unordered(_ParallelGenerateOne, targets): + timings.append(timing) + else: + for target in targets: + timings.append(target.generate()) + + timings.sort(key=lambda t: t.generate_time) + + print(" Time (s) | {:^50} | {:^50}".format("Config", "Template")) + for timing in timings: + tmpl = timing.template + + if len(tmpl) > 50: + # easier to distinguish paths ... shorten common in-fixes + tmpl = tmpl.replace("/zap-templates/", "/../") + tmpl = tmpl.replace("/templates/", "/../") + + print(" %8d | %50s | %50s" % ( + timing.generate_time, + ".." + timing.config[len(timing.config) - + 48:] if len(timing.config) > 50 else timing.config, + ".." + tmpl[len(tmpl) - 48:] if len(tmpl) > 50 else tmpl, + )) if __name__ == '__main__':