From 273722c420e7258d595beed278dbdf259e8cd06c Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Fri, 19 Jan 2024 23:04:25 +0000 Subject: [PATCH] DAOS-12950 test: add ftest tag linter Features: NvmeObject - Add ftest/tags.py utility - Add a lint option for ftest tags - Add infrastructure that can be extended for future tag work Required-githooks: true Signed-off-by: Dalton Bohning --- .github/workflows/linting.yml | 9 + debian/changelog | 6 + src/tests/ftest/nvme/object.py | 10 +- src/tests/ftest/tags.py | 436 +++++++++++++++++++++++++++++++++ utils/rpms/daos.spec | 7 +- 5 files changed, 461 insertions(+), 7 deletions(-) create mode 100755 src/tests/ftest/tags.py diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 9bdef7c96950..c9bd0293d17f 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -58,3 +58,12 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} - name: Check DAOS logging macro use. run: ./utils/cq/d_logging_check.py --github src + + ftest-tags: + name: Ftest tag check + runs-on: ubuntu-22.04 + steps: + - name: Checkout code + uses: actions/checkout@v4 + - name: Check DAOS ftest tags. + run: \[ ! -x src/tests/ftest/tags.py \] || ./src/tests/ftest/tags.py lint diff --git a/debian/changelog b/debian/changelog index c9f8c2e6aa60..3f39d67ebb53 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +daos (2.5.100-14) unstable; urgency=medium + [ Dalton Bohning ] + * Add tags.py + + -- Dalton Bohning Fri, 19 Jan 2024 12:00:00 -0500 + daos (2.5.100-13) unstable; urgency=medium [ Brian J. Murrell ] * Update for EL 8.8 and Leap 15.5 diff --git a/src/tests/ftest/nvme/object.py b/src/tests/ftest/nvme/object.py index 4c473401731e..905a4c974d1f 100644 --- a/src/tests/ftest/nvme/object.py +++ b/src/tests/ftest/nvme/object.py @@ -64,7 +64,7 @@ def container_read(container, array_size=None): # read written objects and verify container.read_objects() - def test_runner(self, namespace, record_size, array_size, thread_per_size=4): + def run_test(self, namespace, record_size, array_size, thread_per_size=4): """Perform simultaneous writes of varying record size to a container. Args: @@ -143,7 +143,7 @@ def test_nvme_object_single_pool(self): :avocado: tags=NvmeObject,test_nvme_object_single_pool """ # perform multiple object writes to a single pool - self.test_runner("/run/pool_1/*", self.record_size[:-1], 0, self.array_size) + self.run_test("/run/pool_1/*", self.record_size[:-1], 0, self.array_size) report_errors(self, self.errors) @avocado.fail_on(DaosApiError) @@ -165,7 +165,7 @@ def test_nvme_object_multiple_pools(self): :avocado: tags=NvmeObject,test_nvme_object_multiple_pools """ # thread to perform simultaneous object writes to multiple pools - runner_manager = ThreadManager(self.test_runner, self.get_remaining_time() - 30) + runner_manager = ThreadManager(self.run_test, self.get_remaining_time() - 30) runner_manager.add( namespace='/run/pool_1/*', record_size=self.record_size, array_size=self.array_size) runner_manager.add( @@ -178,6 +178,6 @@ def test_nvme_object_multiple_pools(self): self.errors.append(result.result) report_errors(self, self.errors) - # run the test_runner after cleaning up all the pools for large nvme_pool size - self.test_runner("/run/pool_3/*", self.record_size, self.array_size) + # run again after cleaning up all the pools for large nvme_pool size + self.run_test("/run/pool_3/*", self.record_size, self.array_size) report_errors(self, self.errors) diff --git a/src/tests/ftest/tags.py b/src/tests/ftest/tags.py new file mode 100755 index 000000000000..f7d98d7890a4 --- /dev/null +++ b/src/tests/ftest/tags.py @@ -0,0 +1,436 @@ +#!/usr/bin/env python3 +""" + (C) Copyright 2023 Intel Corporation. + + SPDX-License-Identifier: BSD-2-Clause-Patent +""" +import ast +import os +import re +import sys +from argparse import ArgumentParser +from collections import defaultdict +from copy import deepcopy +from pathlib import Path + +THIS_FILE = os.path.realpath(__file__) +FTEST_DIR = os.path.dirname(THIS_FILE) +ROOT_DIR = os.path.realpath(os.path.join(FTEST_DIR, '..', '..', '..')) + + +class LintFailure(Exception): + """Exception for lint failures.""" + + +def all_python_files(path): + """Get a list of all .py files recursively in a directory. + + Args: + path (str): directory to look in + + Returns: + list: sorted path names of .py files + """ + return sorted(map(str, Path(path).rglob("*.py"))) + + +class FtestTagMap(): + """Represent tags for ftest/avocado.""" + + def __init__(self, paths): + """Initialize the tag mapping. + + Args: + paths (list): the file or dir path(s) to update from + """ + self.__mapping = {} # str(file_name) : str(class_name) : str(test_name) : set(tags) + for path in paths: + self.__update_from_path(path) + + def __iter__(self): + """Iterate over the mapping. + + Yields: + tuple: file_name, class_name mapping + """ + for item in self.__mapping.items(): + yield deepcopy(item) + + def methods(self): + """Get a mapping of methods to tags. + + Yields: + (str, set): method name and tags + """ + for _, classes in self.__mapping.items(): + for _, methods in classes.items(): + for method_name, tags in methods.items(): + yield (method_name, tags) + + def unique_tags(self, exclude=None): + """Get the set of unique tags, excluding one or more paths. + + Args: + exclude (list, optional): path(s) to exclude from the unique set. + Defaults to None. + + Returns: + set: the set of unique tags + """ + exclude = list(map(self.__norm_path, exclude or [])) + + unique_tags = set() + for file_path, classes in self.__mapping.items(): + if file_path in exclude: + continue + for functions in classes.values(): + for tags in functions.values(): + unique_tags.update(tags) + return unique_tags + + def minimal_tags(self, include_paths=None): + """Get the minimal tags representing files in the mapping. + + This computes an approximate minimal - not the absolute minimal. + + Args: + include_paths (list, optional): path(s) to include in the mapping. + Defaults to None, which includes all paths + + Returns: + list: list of sets of tags + """ + include_paths = list(map(self.__norm_path, include_paths or [])) + + minimal_sets = [] + + for file_path, classes in self.__mapping.items(): + if include_paths and file_path not in include_paths: + continue + # Keep track of recommended tags for each method + file_recommended = [] + for class_name, functions in classes.items(): + for function_name, tags in functions.items(): + # Try the class name and function name first + if class_name in tags: + file_recommended.append(set([class_name])) + continue + if function_name in tags: + file_recommended.append(set([function_name])) + continue + # Try using a set of tags globally unique to this test + globally_unique_tags = tags - self.unique_tags(exclude=file_path) + if globally_unique_tags and globally_unique_tags.issubset(tags): + file_recommended.append(globally_unique_tags) + continue + # Fallback to just using all of this test's tags + file_recommended.append(tags) + + if not file_recommended: + continue + + # If all functions in the file have a common set of tags, use that set + file_recommended_intersection = set.intersection(*file_recommended) + if file_recommended_intersection: + minimal_sets.append(file_recommended_intersection) + continue + + # Otherwise, use tags unique to each function + file_recommended_unique = [] + for tags in file_recommended: + if tags not in file_recommended_unique: + file_recommended_unique.append(tags) + minimal_sets.extend(file_recommended_unique) + + # Combine the minimal sets into a single set representing what avocado expects + avocado_set = set(','.join(tags) for tags in minimal_sets) + + return avocado_set + + def is_test_subset(self, tags1, tags2): + """Determine whether a set of tags is a subset with respect to tests. + + Args: + tags1 (list): list of sets of tags + tags2 (list): list of sets of tags + + Returns: + bool: whether tags1's tests is a subset of tags2's tests + """ + tests1 = set(self.__tags_to_tests(tags1)) + tests2 = set(self.__tags_to_tests(tags2)) + return tests1.issubset(tests2) + + def __tags_to_tests(self, tags): + """Convert a list of tags to the tests they would run. + + Args: + tags (list): list of sets of tags + """ + tests = [] + for method_name, test_tags in self.methods(): + for tag_set in tags: + if tag_set.issubset(test_tags): + tests.append(method_name) + break + return tests + + def __update_from_path(self, path): + """Update the mapping from a path. + + Args: + path (str): the file or dir path to update from + + Raises: + ValueError: if a path is not a file + """ + path = self.__norm_path(path) + + if os.path.isdir(path): + for __path in all_python_files(path): + self.__parse_file(__path) + return + + if os.path.isfile(path): + self.__parse_file(path) + return + + raise ValueError(f'Expected file or directory: {path}') + + def __parse_file(self, path): + """Parse a file and update the internal mapping from avocado tags. + + Args: + path (str): file to parse + """ + with open(path, 'r') as file: + file_data = file.read() + + module = ast.parse(file_data) + for class_def in filter(lambda val: isinstance(val, ast.ClassDef), module.body): + for func_def in filter(lambda val: isinstance(val, ast.FunctionDef), class_def.body): + if not func_def.name.startswith('test_'): + continue + tags = self.__parse_avocado_tags(ast.get_docstring(func_def)) + self.__update(path, class_def.name, func_def.name, tags) + + @staticmethod + def __norm_path(path): + """Convert to "realpath" and replace .yaml paths with .py equivalents. + + Args: + path (str): path to normalize + + Returns: + str: the normalized path + """ + path = os.path.realpath(path) + if path.endswith('.yaml'): + path = re.sub(r'\.yaml$', '.py', path) + return path + + def __update(self, file_name, class_name, test_name, tags): + """Update the internal mapping by appending the tags. + + Args: + file_name (str): file name + class_name (str): class name + test_name (str): test name + tags (set): set of tags to update + """ + if file_name not in self.__mapping: + self.__mapping[file_name] = {} + if class_name not in self.__mapping[file_name]: + self.__mapping[file_name][class_name] = {} + if test_name not in self.__mapping[file_name][class_name]: + self.__mapping[file_name][class_name][test_name] = set() + self.__mapping[file_name][class_name][test_name].update(tags) + + @staticmethod + def __parse_avocado_tags(text): + """Parse avocado tags from a string. + + Args: + text (str): the string to parse for tags + + Returns: + set: the set of tags + """ + tag_strings = re.findall(':avocado: tags=(.*)', text) + if not tag_strings: + return set() + return set(','.join(tag_strings).split(',')) + + +def sorted_tags(tags): + """Get a sorted list of tags. + + Args: + tags (set): original tags + + Returns: + list: sorted tags + """ + tags_tmp = set(tags) + new_tags = [] + for tag in ('all', 'vm', 'hw', 'medium', 'large', 'pr', 'daily_regression', 'full_regression'): + if tag in tags_tmp: + new_tags.append(tag) + tags_tmp.remove(tag) + new_tags.extend(sorted(tags_tmp)) + return new_tags + + +def run_linter(paths=None): + """Run the ftest tag linter. + + Args: + paths (list, optional): paths to lint. Defaults to all ftest python files + + Raises: + LintFailure: if linting fails + """ + if not paths: + paths = all_python_files(FTEST_DIR) + all_files = [] + all_classes = defaultdict(int) + all_methods = defaultdict(int) + test_wo_tags = [] + tests_wo_class_as_tag = [] + tests_wo_method_as_tag = [] + tests_wo_hw_vm_manual = [] + ftest_tag_map = FtestTagMap(paths) + for file_path, classes in iter(ftest_tag_map): + all_files.append(file_path) + for class_name, functions in classes.items(): + all_classes[class_name] += 1 + for method_name, tags in functions.items(): + all_methods[method_name] += 1 + if len(tags) == 0: + test_wo_tags.append(method_name) + if class_name not in tags: + tests_wo_class_as_tag.append(method_name) + if method_name not in tags: + tests_wo_method_as_tag.append(method_name) + if not set(tags).intersection(set(['vm', 'hw', 'manual'])): + tests_wo_hw_vm_manual.append(method_name) + + non_unique_classes = list(name for name, num in all_classes.items() if num > 1) + non_unique_methods = list(name for name, num in all_methods.items() if num > 1) + + print('ftest tag lint') + + def _error_handler(_list, message, required=True): + """Exception handler for each class of failure.""" + _list_len = len(_list) + req_str = '(required)' if required else '(optional)' + print(f' {req_str} {len(_list)} {message}') + if _list_len == 0: + return None + for _test in _list: + print(f' {_test}') + if _list_len > 3: + remaining = _list_len - 3 + _list = _list[:3] + [f"... (+{remaining})"] + _list_str = ", ".join(_list) + if not required: + # Print but do not fail + return None + return LintFailure(f"{_list_len} {message}: {_list_str}") + + # Lint fails if any of the lists contain entries + errors = list(filter(None, [ + _error_handler(non_unique_classes, 'non-unique test classes'), + _error_handler(non_unique_methods, 'non-unique test methods'), + _error_handler(test_wo_tags, 'tests without tags'), + _error_handler(tests_wo_class_as_tag, 'tests without class as tag', required=False), + _error_handler(tests_wo_method_as_tag, 'tests without method name as tag'), + _error_handler(tests_wo_hw_vm_manual, 'tests without HW, VM, or manual tag')])) + if errors: + raise errors[0] + + +def run_dump(paths=None): + """Dump the tags per test. + + Formatted as + : + : + - + + Args: + paths (list, optional): path(s) to get tags for. Defaults to all ftest python files + """ + if not paths: + paths = all_python_files(FTEST_DIR) + for file_path, classes in iter(FtestTagMap(paths)): + short_file_path = re.findall(r'ftest/(.*$)', file_path)[0] + print(f'{short_file_path}:') + for class_name, functions in classes.items(): + print(f' {class_name}:') + all_methods = [] + longest_method_name = 0 + for method_name, tags in functions.items(): + longest_method_name = max(longest_method_name, len(method_name)) + all_methods.append((method_name, tags)) + for method_name, tags in all_methods: + method_name_fm = method_name.ljust(longest_method_name, " ") + tags_fm = ",".join(sorted_tags(tags)) + print(f' {method_name_fm} - {tags_fm}') + + +def files_to_tags(paths): + """Get the unique tags for paths. + + Args: + paths (list): paths to get pragmas for + + Returns: + set: set of test tags representing paths + """ + # Get tags for ftest paths + ftest_tag_map = FtestTagMap(all_python_files(FTEST_DIR)) + ftest_tag_set = ftest_tag_map.minimal_tags(paths) + + # Future work will also get recommended tags for non-ftest files + return ftest_tag_set + + +def run_list(paths): + """List unique tags for paths. + + Args: + paths (list): paths to list tags of + """ + tags = files_to_tags(paths) + print(' '.join(sorted(tags))) + + +if __name__ == '__main__': + parser = ArgumentParser() + parser.add_argument( + "command", + choices=("lint", "list", "dump"), + help="command to run") + parser.add_argument( + "paths", + nargs="*", + help="file paths") + args = parser.parse_args() + args.paths = list(map(os.path.realpath, args.paths)) + + if args.command == "lint": + try: + run_linter(args.paths) + except LintFailure as err: + print(err) + sys.exit(1) + sys.exit(0) + + if args.command == "dump": + run_dump(args.paths) + sys.exit(0) + + if args.command == "list": + run_list(args.paths) + sys.exit(0) diff --git a/utils/rpms/daos.spec b/utils/rpms/daos.spec index d9dce510b126..13745307e87e 100644 --- a/utils/rpms/daos.spec +++ b/utils/rpms/daos.spec @@ -15,7 +15,7 @@ Name: daos Version: 2.5.100 -Release: 13%{?relval}%{?dist} +Release: 14%{?relval}%{?dist} Summary: DAOS Storage Engine License: BSD-2-Clause-Patent @@ -364,7 +364,7 @@ install -m 644 utils/systemd/%{agent_svc_name} %{buildroot}/%{_unitdir} mkdir -p %{buildroot}/%{conf_dir}/certs/clients mv %{buildroot}/%{conf_dir}/bash_completion.d %{buildroot}/%{_sysconfdir} # fixup env-script-interpreters -sed -i -e '1s/env //' %{buildroot}{%{daoshome}/TESTING/ftest/{cart/cart_logtest,config_file_gen,launch,slurm_setup,verify_perms}.py,%{_bindir}/daos_storage_estimator.py,%{_datarootdir}/daos/control/setup_spdk.sh} +sed -i -e '1s/env //' %{buildroot}{%{daoshome}/TESTING/ftest/{cart/cart_logtest,config_file_gen,launch,slurm_setup,tags,verify_perms}.py,%{_bindir}/daos_storage_estimator.py,%{_datarootdir}/daos/control/setup_spdk.sh} # shouldn't have source files in a non-devel RPM rm -f %{buildroot}%{daoshome}/TESTING/ftest/cart/{test_linkage.cpp,utest_{hlc,portnumber,protocol,swim}.c,wrap_cmocka.h} @@ -585,6 +585,9 @@ getent passwd daos_agent >/dev/null || useradd -s /sbin/nologin -r -g daos_agent # No files in a shim package %changelog +* Fri Jan 19 2024 Dalton Bohning 2.5.100-14 +- Add ftest/tags.py + * Wed Dec 06 2023 Brian J. Murrell 2.5.100-13 - Update for EL 8.8 and Leap 15.5 - Update raft to 0.10.1-2.411.gefa15f4