From 7cec6ff3846e9048a233539e47e8854283f233d6 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 17:26:04 +0100 Subject: [PATCH 01/12] bisector: add bisector report -oresult-uuid Allow selecting ResultBundle by UUID. --- tools/bisector/bisector/bisector.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index af363deb2..3d48f3c7c 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -1740,6 +1740,7 @@ class LISATestStep(ShellStep): show_details = ChoiceOrBoolParam(['msg'], 'show details of results. Use "msg" for only a brief message'), show_artifact_dirs = BoolParam('show exekall artifact directory for all iterations'), testcase = CommaListParam('show only the test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name'), + result_uuid = CommaListParam('show only the test results with a UUID in the comma-separated list.'), ignore_testcase = CommaListParam('completely ignore test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name.'), ignore_non_issue = BoolParam('consider only tests that failed or had an error'), ignore_non_error = BoolParam('consider only tests that had an error'), @@ -1910,6 +1911,7 @@ class LISATestStep(ShellStep): show_pass_rate = False, show_artifact_dirs = False, testcase = [], + result_uuid = [], ignore_testcase = [], iterations = [], ignore_non_issue = False, @@ -1941,6 +1943,7 @@ class LISATestStep(ShellStep): out = MLString() considered_testcase_set = set(testcase) + considered_uuid_set = set(result_uuid) ignored_testcase_set = set(ignore_testcase) considered_iteration_set = set(iterations) @@ -1981,6 +1984,10 @@ class LISATestStep(ShellStep): for pattern in considered_testcase_set )) or + (considered_uuid_set and + froz_val.uuid not in considered_uuid_set + ) + or (ignored_testcase_set and any( fnmatch.fnmatch(testcase_id, pattern) for pattern in ignored_testcase_set -- GitLab From 7ccc99f6a831e78bf449e4050abaa2edba3b04c7 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 18:59:35 +0100 Subject: [PATCH 02/12] bisector: refine -otestcase bisector report -otestcase now matches non-tagged testcase IDs, instead of tagged ones. --- tools/bisector/bisector/bisector.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index 3d48f3c7c..95357abf1 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -1739,9 +1739,9 @@ class LISATestStep(ShellStep): show_pass_rate = BoolParam('always show the pass rate of tests, even when there are failures or errors as well'), show_details = ChoiceOrBoolParam(['msg'], 'show details of results. Use "msg" for only a brief message'), show_artifact_dirs = BoolParam('show exekall artifact directory for all iterations'), - testcase = CommaListParam('show only the test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name'), + testcase = CommaListParam('show only the untagged test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name'), result_uuid = CommaListParam('show only the test results with a UUID in the comma-separated list.'), - ignore_testcase = CommaListParam('completely ignore test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name.'), + ignore_testcase = CommaListParam('completely ignore untagged test cases matching one of the patterns in the comma-separated list. * can be used to match any part of the name.'), ignore_non_issue = BoolParam('consider only tests that failed or had an error'), ignore_non_error = BoolParam('consider only tests that had an error'), ignore_excep = CommaListParam('ignore the given comma-separated list of exceptions class name patterns that caused tests error. This will also match on base classes of the exception.'), @@ -1975,12 +1975,12 @@ class LISATestStep(ShellStep): froz_val_list = sorted(db.get_roots(), key=key) for froz_val in froz_val_list: - testcase_id = froz_val.get_id(qual=False, with_tags=True) + untagged_testcase_id = froz_val.get_id(qual=False, with_tags=False) # Ignore tests we are not interested in if ( (considered_testcase_set and not any( - fnmatch.fnmatch(testcase_id, pattern) + fnmatch.fnmatch(untagged_testcase_id, pattern) for pattern in considered_testcase_set )) or @@ -1989,12 +1989,13 @@ class LISATestStep(ShellStep): ) or (ignored_testcase_set and any( - fnmatch.fnmatch(testcase_id, pattern) + fnmatch.fnmatch(untagged_testcase_id, pattern) for pattern in ignored_testcase_set )) ): continue + testcase_id = froz_val.get_id(qual=False, with_tags=True) entry = { 'testcase_id': testcase_id, 'i_stack': i_stack, @@ -2073,6 +2074,7 @@ class LISATestStep(ShellStep): elif entry['result'] == 'passed': # Only change from None to GOOD but not from BAD to GOOD bisect_ret = BisectRet.GOOD if bisect_ret is None else bisect_ret + testcase_map.setdefault(testcase_id, []).append(entry) any_entries = bisect_ret is not None -- GitLab From f21fc9a460aa0993d86e1d9b4144c516b5c3b778 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 18:35:40 +0100 Subject: [PATCH 03/12] doc: update manpage --- doc/man1/bisector.1 | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/doc/man1/bisector.1 b/doc/man1/bisector.1 index b2550a019..5689941c1 100644 --- a/doc/man1/bisector.1 +++ b/doc/man1/bisector.1 @@ -353,13 +353,17 @@ Inherits from: shell consider only tests that failed or had an error \-o ignore\-testcase= (comma\-separated list) - completely ignore test cases matching one of the patterns in the - comma\-separated list. * can be used to match any part of the name. + completely ignore untagged test cases matching one of the patterns + in the comma\-separated list. * can be used to match any part of the + name. \-o iterations= (comma\-separated list of integer ranges) comma\-separated list of iterations to consider. Inclusive ranges can be specified with \- + \-o result\-uuid= (comma\-separated list) + show only the test results with a UUID in the comma\-separated list. + \-o show\-artifact\-dirs= (bool) show exekall artifact directory for all iterations @@ -383,8 +387,9 @@ Inherits from: shell tests \-o testcase= (comma\-separated list) - show only the test cases matching one of the patterns in the comma\- - separated list. * can be used to match any part of the name + show only the untagged test cases matching one of the patterns in + the comma\-separated list. * can be used to match any part of the + name \-o upload\-artifact= (bool) upload the artifact directory to Artifactorial and update the in\- -- GitLab From e0281653f97705669bdfce8fdfdbb4c094b5f4b1 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 18:28:45 +0100 Subject: [PATCH 04/12] bisector: refine -oexport-db bisector report -oexport-db now outputs an exekall ValueDB that only contains the root values that were selected using the other options. --- tools/bisector/bisector/bisector.py | 35 ++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index 95357abf1..9ff74a474 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -1950,7 +1950,6 @@ class LISATestStep(ShellStep): # Read the ValueDB from exekall to know the failing tests testcase_map = dict() filtered_step_res_seq = list() - db_list = [] for step_res_item in step_res_seq: i_stack, step_res = step_res_item bisect_ret = None @@ -1967,7 +1966,6 @@ class LISATestStep(ShellStep): )) continue else: - db_list.append(db) def key(froz_val): return froz_val.get_id(full_qual=True, with_tags=True) @@ -2063,6 +2061,7 @@ class LISATestStep(ShellStep): is_ignored = True entry['froz_val'] = froz_val + entry['db'] = db # Ignored testcases will not contribute to the number of # iterations @@ -2323,11 +2322,35 @@ class LISATestStep(ShellStep): # Write-out a merged DB if export_db: - try: + + entry_list = [ + entry + for entry_list in testcase_map.values() + for entry in entry_list + ] + + # Prune the DBs so we only keep the root values we selected + # previously and all its parents. + def get_parents_uuid(froz_val): + yield froz_val.uuid + for parent_froz_val in froz_val.values(): + yield from get_parents_uuid(parent_froz_val) + + allowed_uuids = set(itertools.chain.from_iterable( + get_parents_uuid(entry['froz_val']) + for entry in entry_list + )) + + def prune_predicate(froz_val): + return froz_val.uuid not in allowed_uuids + + db_list = [ + db.prune_by_predicate(prune_predicate) + for db in {entry['db'] for entry in entry_list} + ] + + with contextlib.suppress(FileNotFoundError): existing_db = ValueDB.from_path(export_db) - except FileNotFoundError: - pass - else: db_list.append(existing_db) merged_db = ValueDB.merge(db_list) -- GitLab From c212da802753bd495ac6f6ac020e5f320ba0471d Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 24 Apr 2019 14:25:19 +0100 Subject: [PATCH 05/12] exekall.engine: Fix typo in FrozenExprVal.type_names s/value/self.value/ --- tools/exekall/exekall/engine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index 24b9e8023..bf6597302 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -3139,7 +3139,7 @@ class FrozenExprVal(ExprValBase): def type_names(self): return [ utils.get_name(type_, full_qual=True) - for type_ in utils.get_mro(type(value)) + for type_ in utils.get_mro(type(self.value)) if type_ is not object ] -- GitLab From 6f65c78d8ba1607af9cc9883194802bb84ddc9d0 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 11:40:38 +0100 Subject: [PATCH 06/12] exekall: merge: Fix merging of a VALUE_DB.pickle.xz files Do not always assume the output is a directory. --- tools/exekall/exekall/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/exekall/exekall/main.py b/tools/exekall/exekall/main.py index 408da902e..a8fb2e170 100755 --- a/tools/exekall/exekall/main.py +++ b/tools/exekall/exekall/main.py @@ -479,10 +479,10 @@ def do_merge(artifact_dirs, output_dir, use_hardlink=True, output_exist=False): merged_db_path = output_dir else: # This will fail loudly if the folder already exists - os.makedirs(str(output_dir), exist_ok=output_exist) + output_dir.mkdir(parents=True, exist_ok=output_exist) + (output_dir/'BY_UUID').mkdir(exist_ok=True) merged_db_path = output_dir/DB_FILENAME - (output_dir/'BY_UUID').mkdir(exist_ok=True) testsession_uuid_list = [] for artifact_dir in artifact_dirs: with (artifact_dir/'UUID').open(encoding='utf-8') as f: -- GitLab From d5691405a2e7262fa4589aaea78e773805b25439 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 11:47:08 +0100 Subject: [PATCH 07/12] exekall: Move DB_FILENAME in _utils.py --- tools/exekall/exekall/_utils.py | 2 ++ tools/exekall/exekall/main.py | 12 +++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/exekall/exekall/_utils.py b/tools/exekall/exekall/_utils.py index b85fac8c2..6d8946500 100644 --- a/tools/exekall/exekall/_utils.py +++ b/tools/exekall/exekall/_utils.py @@ -38,6 +38,8 @@ import glob import textwrap import argparse +DB_FILENAME = 'VALUE_DB.pickle.xz' + class NotSerializableError(Exception): pass diff --git a/tools/exekall/exekall/main.py b/tools/exekall/exekall/main.py index a8fb2e170..7ba064b0b 100755 --- a/tools/exekall/exekall/main.py +++ b/tools/exekall/exekall/main.py @@ -36,8 +36,6 @@ import exekall.utils as utils from exekall.utils import NoValue, error, warn, debug, info, out, add_argument import exekall.engine as engine -DB_FILENAME = 'VALUE_DB.pickle.xz' - # Create an operator for all callables that have been detected in a given # set of modules def build_op_set(callable_pool, non_reusable_type_set, allowed_pattern_set, adaptor): @@ -481,7 +479,7 @@ def do_merge(artifact_dirs, output_dir, use_hardlink=True, output_exist=False): # This will fail loudly if the folder already exists output_dir.mkdir(parents=True, exist_ok=output_exist) (output_dir/'BY_UUID').mkdir(exist_ok=True) - merged_db_path = output_dir/DB_FILENAME + merged_db_path = output_dir/utils.DB_FILENAME testsession_uuid_list = [] for artifact_dir in artifact_dirs: @@ -551,7 +549,7 @@ def do_merge(artifact_dirs, output_dir, use_hardlink=True, output_exist=False): else: shutil.copy2(str(path), str(dst_path)) - if dirpath == artifact_dir and name == DB_FILENAME: + if dirpath == artifact_dir and name == utils.DB_FILENAME: db_path_list.append(path) if artifact_dirs: @@ -954,7 +952,7 @@ def exec_expr_list(iteration_expr_list, adaptor, artifact_dir, testsession_uuid, f.write( expr.get_script( prefix = 'expr', - db_path = os.path.join('..', DB_FILENAME), + db_path = os.path.join('..', utils.DB_FILENAME), db_relative_to = '__file__', )[1]+'\n', ) @@ -1079,7 +1077,7 @@ def exec_expr_list(iteration_expr_list, adaptor, artifact_dir, testsession_uuid, f.write( expr.get_script( prefix = 'expr', - db_path = os.path.join('..', '..', DB_FILENAME), + db_path = os.path.join('..', '..', utils.DB_FILENAME), db_relative_to = '__file__', )[1]+'\n', ) @@ -1117,7 +1115,7 @@ def exec_expr_list(iteration_expr_list, adaptor, artifact_dir, testsession_uuid, adaptor_cls=adaptor_cls, ) - db_path = artifact_dir/DB_FILENAME + db_path = artifact_dir/utils.DB_FILENAME db.to_path(db_path) out('#'*80) -- GitLab From 62cb68bee5e6f63f303d1c1616e7281905f07af8 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 23 Apr 2019 18:53:45 +0100 Subject: [PATCH 08/12] exekall: Add remove_tags param to get_id() Allows not showing a given tag. --- lisa/regression.py | 7 +------ tools/exekall/exekall/engine.py | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lisa/regression.py b/lisa/regression.py index 463e413f4..4c28d7e03 100644 --- a/lisa/regression.py +++ b/lisa/regression.py @@ -221,14 +221,9 @@ def compute_regressions(old_list, new_list, remove_tags=[], **kwargs): new_list = dedup_list(new_list, old_list) def get_id(froz_val): - id_ = froz_val.get_id(qual=False, with_tags=True) - # Remove tags, so that more test will share the same ID. This allows # cross-board comparison for example. - for tag in remove_tags: - id_ = re.sub(r'\[{}=.*?\]'.format(tag), '', id_) - - return id_ + return froz_val.get_id(qual=False, with_tags=True, remove_tags=remove_tags) def group_by_testcase(froz_val_list): return OrderedDict( diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index bf6597302..d614f4e1c 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -28,6 +28,7 @@ import contextlib import pickle import pprint import pickletools +import re import exekall._utils as utils from exekall._utils import NoValue @@ -776,6 +777,9 @@ class ExpressionBase(ExprHelpers): :class:`ExprVal`. :type with_tags: bool + :param remove_tags: Do not add the specified tags values. + :type remove_tags: set(str) + :param qual: If True, return the qualified ID. :type qual: bool @@ -800,7 +804,7 @@ class ExpressionBase(ExprHelpers): else: return id_ - def _get_id(self, with_tags=True, full_qual=True, qual=True, style=None, expr_val=None, marked_expr_val_set=None, hidden_callable_set=None): + def _get_id(self, with_tags=True, remove_tags=set(), full_qual=True, qual=True, style=None, expr_val=None, marked_expr_val_set=None, hidden_callable_set=None): if hidden_callable_set is None: hidden_callable_set = set() @@ -819,6 +823,7 @@ class ExpressionBase(ExprHelpers): param_map=param_map, expr_val=expr_val, with_tags=with_tags, + remove_tags=remove_tags, marked_expr_val_set=marked_expr_val_set, hidden_callable_set=hidden_callable_set, full_qual=full_qual, @@ -826,7 +831,7 @@ class ExpressionBase(ExprHelpers): style=style, ) - def _get_id_internal(self, param_map, expr_val, with_tags, marked_expr_val_set, hidden_callable_set, full_qual, qual, style): + def _get_id_internal(self, param_map, expr_val, with_tags, remove_tags, marked_expr_val_set, hidden_callable_set, full_qual, qual, style): separator = ':' marker_char = '^' get_id_kwargs = dict( @@ -844,6 +849,7 @@ class ExpressionBase(ExprHelpers): (param, param_expr._get_id( **get_id_kwargs, with_tags = with_tags, + remove_tags=remove_tags, # Pass None when there is no value available, so we will get # a non-tagged ID when there is no value computed expr_val = param_map.get(param), @@ -861,7 +867,7 @@ class ExpressionBase(ExprHelpers): def get_tags(expr_val): if expr_val is not None: if with_tags: - tag = expr_val.format_tags() + tag = expr_val.format_tags(remove_tags) else: tag = '' return tag @@ -3208,7 +3214,7 @@ class FrozenExprVal(ExprValBase): def _make_id_key(**kwargs): return tuple(sorted(kwargs.items())) - def get_id(self, full_qual=True, qual=True, with_tags=True): + def get_id(self, full_qual=True, qual=True, with_tags=True, remove_tags=set()): """ Return recorded IDs generated using :meth:`ExprVal.get_id`. """ @@ -3218,7 +3224,12 @@ class FrozenExprVal(ExprValBase): qual=qual, with_tags=with_tags ) - return self.recorded_id_map[key] + id_ = self.recorded_id_map[key] + + for tag in remove_tags: + id_ = re.sub(r'\[{}=.*?\]'.format(tag), '', id_) + + return id_ class PrunedFrozVal(FrozenExprVal): """ @@ -3321,9 +3332,12 @@ class ExprVal(ExprValBase): self.expr = expr super().__init__(param_map=param_map, value=value, excep=excep) - def format_tags(self): + def format_tags(self, remove_tags=set()): """ Return a formatted string for the tags of that :class:`ExprVal`. + + :param remove_tags: Do not include those tags + :type remove_tags: set(str) """ tag_map = { # Make sure there are no brackets in tag values, since that @@ -3335,6 +3349,7 @@ class ExprVal(ExprValBase): return ''.join( '[{}={}]'.format(k, v) if k else '[{}]'.format(v) for k, v in sorted(tag_map.items()) + if k not in remove_tags ) else: return '' -- GitLab From ca442f2e50d71abcf72e4e62a8a746bc62ba54b8 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 24 Apr 2019 14:51:34 +0100 Subject: [PATCH 09/12] exekall: optimize ValueDB creation --- tools/exekall/exekall/engine.py | 74 ++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index d614f4e1c..a854c725e 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -97,12 +97,20 @@ class ValueDB: """ # First pass: find all frozen values corresponding to a given UUID - uuid_map = {} + uuid_map = collections.defaultdict(set) def update_uuid_map(froz_val): - uuid_map.setdefault(froz_val.uuid, set()).add(froz_val) + uuid_map[froz_val.uuid].add(froz_val) return froz_val cls._froz_val_dfs(froz_val_seq_list, update_uuid_map) + # If there is no more than one FrozenExprVal per UUID, we can skip the + # rewriting + for froz_val_set in uuid_map.values(): + if len(froz_val_set) > 1: + break + else: + return froz_val_seq_list + # Select one FrozenExprVal for each UUID pair def select_froz_val(froz_val_set): candidates = [ @@ -135,7 +143,7 @@ class ValueDB: return froz_val return uuid_map[froz_val.uuid] - return cls._froz_val_dfs(froz_val_seq_list, rewrite_graph) + return cls._froz_val_dfs(froz_val_seq_list, rewrite_graph, rewrite=True) @classmethod def merge(cls, db_list, roots_from=None): @@ -288,29 +296,47 @@ class ValueDB: return uuid_map @classmethod - def _froz_val_dfs(cls, froz_val_seq_list, callback): - return [ - FrozenExprValSeq( - froz_val_list=[ - cls._do_froz_val_dfs(froz_val, callback) - for froz_val in froz_val_seq - ], - param_map=OrderedDict( - (param, cls._do_froz_val_dfs(froz_val, callback)) - for param, froz_val in froz_val_seq.param_map.items() + def _froz_val_dfs(cls, froz_val_seq_list, callback, rewrite=False): + if rewrite: + def _do_froz_val_dfs(froz_val): + updated_froz_val = callback(froz_val) + updated_froz_val.param_map = OrderedDict( + (param, _do_froz_val_dfs(param_froz_val)) + for param, param_froz_val in updated_froz_val.param_map.items() ) - ) - for froz_val_seq in froz_val_seq_list - ] - @classmethod - def _do_froz_val_dfs(cls, froz_val, callback): - updated_froz_val = callback(froz_val) - updated_froz_val.param_map = OrderedDict( - (param, cls._do_froz_val_dfs(param_froz_val, callback)) - for param, param_froz_val in updated_froz_val.param_map.items() - ) - return updated_froz_val + return updated_froz_val + + return [ + FrozenExprValSeq( + froz_val_list=[ + _do_froz_val_dfs(froz_val) + for froz_val in froz_val_seq + ], + param_map=OrderedDict( + (param, _do_froz_val_dfs(froz_val)) + for param, froz_val in froz_val_seq.param_map.items() + ) + ) + for froz_val_seq in froz_val_seq_list + ] + + # When we don't need to rewrite the graph, just call the callback so we + # avoid creating loads of useless objects + else: + def _do_froz_val_dfs(froz_val): + callback(froz_val) + for parent in froz_val.param_map.values(): + _do_froz_val_dfs(parent) + + for froz_val_seq in froz_val_seq_list: + for froz_val in froz_val_seq: + _do_froz_val_dfs(froz_val) + + for froz_val in froz_val_seq.param_map.values(): + _do_froz_val_dfs(froz_val) + + return None def get_by_uuid(self, uuid): """ -- GitLab From 105c5464397f7f7b4576b448771cc516d2094d51 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 24 Apr 2019 15:24:59 +0100 Subject: [PATCH 10/12] exekall: Optimize engine.ValueDB.prune_by_predicate() Avoid calling the predicate more than necessary. --- tools/exekall/exekall/engine.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index a854c725e..e82359056 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -431,22 +431,25 @@ class ValueDB: This allows trimming a :class:`ValueDB` to a smaller size by removing non-necessary content. """ - def prune(froz_val): + def prune(froz_val, do_prune): if isinstance(froz_val, PrunedFrozVal): return froz_val - elif predicate(froz_val): + elif do_prune: return PrunedFrozVal(froz_val) else: # Edit the param_map in-place, so we keep it potentially shared # if possible. for param, param_froz_val in list(froz_val.param_map.items()): - froz_val.param_map[param] = prune(param_froz_val) + froz_val.param_map[param] = prune( + param_froz_val, + do_prune=predicate(param_froz_val) + ) return froz_val def make_froz_val_seq(froz_val_seq): froz_val_list = [ - prune(froz_val) + prune(froz_val, do_prune=False) for froz_val in froz_val_seq # Just remove the root PrunedFrozVal, since they are useless at # this level (i.e. nothing depends on them) -- GitLab From 4fbecf6a22f10add08e9e2c6ef3646c2b79d8dff Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 8 Apr 2019 17:22:02 +0100 Subject: [PATCH 11/12] tools: Add check-test-fix.py Add a tool to re-run a given test against data collected using bisector, like for the fortnightly integration. It will show a regression/improvement table at the end to compare the reference data in the bisector report with the new ones that were just generated. It can be called as follow to check that a fix to a RampDown:test_task_placement has been fixed on the first 10 available traces: $ check-test-fix.py -s 'RampDown:test_task_placement' bisector_report.yml.gz -oiterations=1-10 --only behaviour Note: this will spawn one `exekall run` per CPU to speed up the computation. That means the logs are output only when `exekall run` command finishes to avoid interleaved output. --- tools/check-test-fix.py | 353 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 353 insertions(+) create mode 100755 tools/check-test-fix.py diff --git a/tools/check-test-fix.py b/tools/check-test-fix.py new file mode 100755 index 000000000..81c1c2090 --- /dev/null +++ b/tools/check-test-fix.py @@ -0,0 +1,353 @@ +#! /usr/bin/env python3 +# +# SPDX-License-Identifier: Apache-2.0 +# +# Copyright (C) 2019, Arm Limited and contributors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import argparse +import collections +import os +import subprocess +import sys +import tarfile +import multiprocessing.pool +import glob +import pickle +import shutil +import contextlib +from pathlib import Path +import traceback + +from exekall.utils import DB_FILENAME + +def print_exception(e): + traceback.print_exception(type(e), e, e.__traceback__) + +def stringify(seq): + return [str(x) for x in seq] + +def cleanup(path): + """ + Remove a folder of file. + """ + _path = str(path) + with contextlib.suppress(FileNotFoundError): + try: + shutil.rmtree(_path) + except NotADirectoryError: + os.unlink(_path) + return path + +def check_output(*args, **kwargs): + try: + return subprocess.check_output(*args, **kwargs) + except subprocess.CalledProcessError as e: + for out in (e.stderr, e.stdout): + if out: + print(out.decode('utf-8')) + raise + +def download_artifacts(bisector_report, log_folder, exekall_db, extra_opts): + subprocess.call([ + 'bisector', 'report', bisector_report, + '-oexport-logs={}'.format(log_folder), + '-oexport-db={}'.format(exekall_db), + '-odownload', + *stringify(extra_opts), + ]) + +def extract_tar(tar, dest): + """ + Tar extraction worker. + + Only extracts files not already extracted. + """ + + with tarfile.open(str(tar)) as f: + member_names = f.getnames() + root = Path(os.path.commonpath(member_names)) + # Only extract once if the file has already been downloaded and + # extracted before to save IO bandwith + if not all((dest/name).exists() for name in member_names): + f.extractall(str(dest)) + + return dest/root + +def exekall_run(artifact_dir, db_path, test_patterns, test_src_list, log_level, load_type, replay_uuid): + """ + exekall run worker + """ + cmd = [ + 'exekall', 'run', *stringify(test_src_list), + '--load-db', str(db_path), '--artifact-dir', str(artifact_dir), + ] + if replay_uuid: + cmd.extend(['--replay', replay_uuid]) + else: + cmd.extend([ + '--load-type', load_type, + *('--select={}'.format(x) for x in test_patterns) + ]) + + if log_level: + cmd += ['--log-level', log_level] + + # Capture output so it is not interleaved since multiple workers are + # running at the same time. + out = check_output(cmd, stderr=subprocess.STDOUT) + + return artifact_dir, out.decode('utf-8') + +def exekall_merge(merged_artifact, db_path_list): + subprocess.check_call([ + 'exekall', 'merge', + '-o', str(merged_artifact), + *(str(path.parent) for path in db_path_list), + ]) + + return merged_artifact + +def exekall_compare(ref_db_path, new_db_path): + out = check_output([ + 'exekall', 'compare', + str(ref_db_path), str(new_db_path), + '--non-significant', + '--remove-tag', 'board', + ]) + return out.decode('utf-8') + +def main(argv): + parser = argparse.ArgumentParser(description=""" +Re-execute a LISA test on a artifacts referenced by a bisector report, and +show the summary of changes in the results. + +All `bisector` options need to come last on the command line. + +EXAMPLE: + Re-execute all the "test_task_placement" tests on the 10 first iterations + of a hikey960 integration test. + $ check-test-fix.py -s '*:test_task_placement' hikey960.report.yml.gz --cache --only behaviour -oiterations=1-10 +""", + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + + load_group = parser.add_mutually_exclusive_group(required=True) + load_group.add_argument('-s', '--select', + action='append', + metavar='EXEKALL_SELECT_PATTERN', + help='Exekall run --select pattern to re-execute', + ) + + load_group.add_argument('--replay', + metavar='EXEKALL_RESULT_UUID', + help='See exekall run --replay and bisector report -oresult-uuid', + ) + + parser.add_argument('-t', '--python-src', + metavar='EXEKALL_PYTHON_SOURCES', + action='append', + default=[os.path.expandvars('$LISA_HOME/lisa/tests')], + help='Exekall run python modules', + ) + + parser.add_argument('--log-level', + action='store_true', + help='See exekall run --log-level', + ) + + parser.add_argument('--load-type', + default='*TestBundle', + help='See `exekall run --load-type`. It is ignored if --replay is used.', + ) + + parser.add_argument('bisector_report', + metavar='BISECTOR_REPORT', + help='Bisector report to use', + ) + + parser.add_argument('bisector_options', + metavar='BISECTOR_EXTRA_OPTIONS', + nargs=argparse.REMAINDER, + default=[], + help='Extra bisector options', + ) + + parser.add_argument('-j', + metavar='NR_CPUS', + type=int, + default=os.cpu_count() + 2, + help="""Number of processes to use for exekall run and tar extraction. + Defaults to number of CPUs + 2""", + ) + + parser.add_argument('-w', '--work-area', + type=Path, + help="Work area to use when processing the artifacts", + ) + + args = parser.parse_args(argv) + + nr_cpus = args.j + nr_cpus = nr_cpus if nr_cpus > 0 else 1 + bisector_extra_opts = args.bisector_options + result_uuid = args.replay + if result_uuid: + bisector_extra_opts.append('-oresult-uuid={}'.format(result_uuid)) + bisector_report = args.bisector_report + exekall_test_patterns = args.select + if exekall_test_patterns: + bisector_extra_opts.append('-otestcase={}'.format( + ','.join(exekall_test_patterns) + )) + + exekall_python_srcs = args.python_src + exekall_log_level = args.log_level + exekall_load_type = args.load_type + exekall_replay_uuid = result_uuid + + work_area = args.work_area or Path('{}.check-test-fix'.format(Path(bisector_report).name)) + work_area.mkdir(exist_ok=True) + + # Read-back the options used the previous time. If they are the same, + # no need to re-download an extract everything. + extraction_state_path = str(work_area/'extraction_state') + try: + with open(extraction_state_path, 'rb') as f: + extraction_state = pickle.load(f) + except FileNotFoundError: + need_download = True + # If bisector options are different, we need to download and extract again + else: + need_download = (extraction_state['bisector_opts'] != bisector_extra_opts) + + if need_download: + log_folder = work_area/'bisector_logs' + ref_db_path = work_area/'reference_VALUE_DB.pickle.xz' + + # Make sure we don't have any archives from previous download, to avoid + # selecting more archives than we want when listing the content of the + # folder. + cleanup(log_folder) + + download_artifacts(bisector_report, log_folder, ref_db_path, + bisector_extra_opts) + else: + print('Reusing previously downloaded artifacts ...') + ref_db_path = extraction_state['ref_db_path'] + extracted_artifacts = extraction_state['extracted_artifacts'] + log_folder = None + + + # List of ValueDB paths that are freshly created by exekall run. + # Since it is accessed by the pool's callback thread, use an atomic deque + new_db_path_deque = collections.deque() + + # Use the context manager to make sure all worker subprocesses are + # terminated + with multiprocessing.pool.Pool(processes=nr_cpus) as exekall_pool: + + def exekall_run_callback(run_result): + artifact_dir, out = run_result + # Since all callbacks are called from the same thread, + new_db_path_deque.append(artifact_dir/DB_FILENAME) + + # Show the log of the run in one go, to avoid interleaved output + print('#'*80) + print('New results for {}'.format(artifact_dir)) + print('\n\n') + print(out) + + def schedule_exekall_run(artifact_dir): + # schedule an `exekall run` job + new_artifact_dir = cleanup(work_area/'new_exekall_artifacts'/artifact_dir.name) + exekall_pool.apply_async(exekall_run, + kwds=dict( + artifact_dir=new_artifact_dir, + db_path=artifact_dir/DB_FILENAME, + test_patterns=exekall_test_patterns, + test_src_list=exekall_python_srcs, + log_level=exekall_log_level, + load_type=exekall_load_type, + replay_uuid=exekall_replay_uuid, + ), + callback=exekall_run_callback, + ) + + # Uncompress all downloaded archives and kick off exekall run from the + # callback + if need_download: + # Use a deque since it is appended from a different thread + extracted_artifacts = collections.deque() + + def untar_callback(artifact_dir): + extracted_artifacts.append(artifact_dir) + return schedule_exekall_run(artifact_dir) + + # extract archives asynchronously + with multiprocessing.pool.Pool(processes=nr_cpus) as untar_pool: + for tar in glob.iglob( + str(log_folder/'**'/'lisa_artifacts.tar.xz'), + recursive=True + ): + untar_pool.apply_async(extract_tar, + kwds=dict( + tar=tar, + dest=work_area/'extracted_exekall_artifacts' + ), + callback=untar_callback, + error_callback=print_exception, + ) + + untar_pool.close() + untar_pool.join() + # Kick of exekall run + else: + for artifact_dir in extracted_artifacts: + schedule_exekall_run(artifact_dir) + + # Save what was extracted for future reference when we know all the + # archives have been extracted successfully + with open(extraction_state_path, 'wb') as f: + pickle.dump({ + 'bisector_opts': bisector_extra_opts, + 'ref_db_path': ref_db_path, + 'extracted_artifacts': list(extracted_artifacts), + }, + f + ) + + # We know that once all archives has been uncompressed, all exekall run + # jobs have been submitted + exekall_pool.close() + exekall_pool.join() + + # Merge DB before comparison + print('Merging the new artifacts for comparison ...') + merged_artifact = cleanup(work_area/'merged_new_exekall_artifacts') + + print('Comparing the results ...') + merged_artifact_dir = exekall_merge(merged_artifact, new_db_path_deque) + + out = exekall_compare(ref_db_path, merged_artifact_dir/DB_FILENAME) + print('\n\nRegressions/improvements:') + print(out) + + +if __name__ == '__main__': + ret = main(sys.argv[1:]) + sys.exit(ret) + +# vim :set tabstop=4 shiftwidth=4 textwidth=80 expandtab -- GitLab From 0de1c6fc9c2e13722579bde3dd7ff5fbb9b6d4da Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 24 Apr 2019 14:05:22 +0100 Subject: [PATCH 12/12] doc: workflows/automated_testing: Add "fixing regressions" Add check-test-fix.py to the automated_testing workflow description. --- doc/workflows/automated_testing.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/doc/workflows/automated_testing.rst b/doc/workflows/automated_testing.rst index c58c3a6aa..4a3b7718a 100644 --- a/doc/workflows/automated_testing.rst +++ b/doc/workflows/automated_testing.rst @@ -444,6 +444,25 @@ collected ones. This can then be compared with another one for regressions: without relying on other files/folders being available (locally or over HTTP). +Fixing regressions +------------------ + +``check-test-fix.py`` tool can be used to check that a fix to a test resolved +errors or a regression, provided that the test can be re-executed on +already-collected :class:`lisa.tests.base.TestBundle` instances. It will call +``exekall run`` in parallel on all the ``exekall``'s +:class:`exekall.engine.ValueDB` collected by ``bisector run``, and will produce +a regression table using ``exekall compare`` with ``old`` being the results +from the report, and ``new`` being the new results. + +.. code-block:: sh + + # The test to check is selected using --select in the same way as for `exekall run`. + # hikey960.report.yml.gz is a bisector report generated using `bisector run` + # All options coming after the report are passed to `bisector report` to + # control what artifacts are downloaded and what TestBundle are used. + check-test-fix.py --select 'OneSmallTask:test_task_placement' hikey960.report.yml.gz -oiterations=1-20 + When something goes wrong +++++++++++++++++++++++++ -- GitLab