From ee57f30281a71689f18639617fb30da889de4fce Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Thu, 24 Aug 2023 16:11:08 +0800 Subject: [PATCH] a revision on test naming - fix tracing tests mt name output - fix xfail tests st name matching - fix task reassign check - some refactor & typing fix --- testplan/common/report/base.py | 30 ++++++++------ testplan/common/report/schemas.py | 1 + testplan/common/utils/interface.py | 2 +- .../exporters/testing/coverage/__init__.py | 16 +++++--- .../testing/pdf/renderers/reports.py | 18 +++++---- testplan/parser.py | 11 +++++ testplan/runnable/base.py | 24 ++++------- testplan/runnable/interactive/base.py | 15 ++++--- testplan/runners/pools/tasks/base.py | 19 ++++----- testplan/testing/base.py | 8 ++-- testplan/testing/multitest/base.py | 15 ++++--- testplan/testing/multitest/parametrization.py | 36 ++++++++--------- testplan/testing/multitest/suite.py | 22 +++------- testplan/web_ui/testing/src/Nav/NavList.js | 2 +- .../testplan/coverage/test_trace_tests.py | 40 ++++++++++++++++--- .../testplan/testing/multitest/test_xfail.py | 6 ++- .../unit/testplan/common/report/test_base.py | 15 ++++--- 17 files changed, 155 insertions(+), 125 deletions(-) diff --git a/testplan/common/report/base.py b/testplan/common/report/base.py index 5dc5caad5..c9cd83003 100644 --- a/testplan/common/report/base.py +++ b/testplan/common/report/base.py @@ -7,16 +7,16 @@ Later on these reports would be merged together to build the final report as the testplan result. """ -import copy -import time -import uuid import collections +import copy import dataclasses import itertools - +import time +import uuid from typing import Dict, List, Optional, Union from testplan.common.utils import strings + from .log import create_logging_adapter @@ -57,11 +57,17 @@ class Report: exception_logger = ExceptionLogger def __init__( - self, name, description=None, uid=None, entries=None, parent_uids=None + self, + name: str, + description: Optional[str] = None, + instance_name: Optional[str] = None, + uid: Optional[str] = None, + entries: Optional[list] = None, + parent_uids: Optional[List[str]] = None, ): self.name = name self.description = description - + self.instance_name = instance_name or name self.uid = uid or name self.entries = entries or [] @@ -77,12 +83,12 @@ def __init__( self.parent_uids = parent_uids or [] def __str__(self): - return '{kls}(name="{name}", id="{uid}")'.format( + return '{kls}(name="{name}", uid="{uid}")'.format( kls=self.__class__.__name__, name=self.name, uid=self.uid ) def __repr__(self): - return '{kls}(name="{name}", id="{uid}", entries={entries})'.format( + return '{kls}(name="{name}", uid="{uid}", entries={entries})'.format( kls=self.__class__.__name__, name=self.name, uid=self.uid, @@ -131,15 +137,15 @@ def logged_exceptions(self, *exception_classes, **kwargs): def _check_report(self, report): """ - Utility method for checking `report` `type` and `uid`. + Utility method for checking `report` `type` and `instance_name`. """ msg = "Report check failed for `{}` and `{}`. ".format(self, report) - if report.uid != self.uid: + if report.instance_name != self.instance_name: raise AttributeError( msg - + "`uid` attributes (`{}`, `{}`) do not match.".format( - self.uid, report.uid + + "`instance_name` attributes (`{}`, `{}`) do not match.".format( + self.instance_name, report.instance_name ) ) diff --git a/testplan/common/report/schemas.py b/testplan/common/report/schemas.py index a814bc276..3f897c8e9 100644 --- a/testplan/common/report/schemas.py +++ b/testplan/common/report/schemas.py @@ -36,6 +36,7 @@ class Meta: name = fields.String() description = fields.String(allow_none=True) + instance_name = fields.String(allow_none=True) # otherwise new tpr cannot process old report entries = fields.List(custom_fields.NativeOrPretty()) parent_uids = fields.List(fields.String()) diff --git a/testplan/common/utils/interface.py b/testplan/common/utils/interface.py index 68e9aa293..ddbdeb6e5 100644 --- a/testplan/common/utils/interface.py +++ b/testplan/common/utils/interface.py @@ -1,7 +1,7 @@ """Validates methods signature.""" from inspect import signature -from typing import Union, Optional +from typing import Union class NoSuchMethodInClass(Exception): diff --git a/testplan/exporters/testing/coverage/__init__.py b/testplan/exporters/testing/coverage/__init__.py index 0de7b9600..a21e5bce5 100644 --- a/testplan/exporters/testing/coverage/__init__.py +++ b/testplan/exporters/testing/coverage/__init__.py @@ -7,15 +7,16 @@ import sys from collections import OrderedDict from contextlib import contextmanager -from typing import Dict, Generator, Mapping, TextIO, Tuple, Optional +from typing import Dict, Generator, Mapping, Optional, TextIO, Tuple from testplan.common.exporters import ( - ExporterConfig, ExportContext, + ExporterConfig, verify_export_context, ) from testplan.exporters.testing.base import Exporter from testplan.report.testing.base import ( + ReportCategories, TestCaseReport, TestGroupReport, TestReport, @@ -52,8 +53,11 @@ def export( # here we use an OrderedDict as an ordered set results = OrderedDict() for entry in source.entries: - if isinstance(entry, TestGroupReport): - self._append_covered_group_n_case(entry, [], results) + if ( + isinstance(entry, TestGroupReport) + and entry.category == ReportCategories.MULTITEST + ): + self._append_covered_group_n_case(entry, tuple(), results) if results: with _custom_open(self.cfg.tracing_tests_output) as ( f, @@ -80,7 +84,7 @@ def _append_covered_group_n_case( Here we use an OrderedDict as an ordered set. """ - curr_path = (*path, report.name) + curr_path = (*path, report.instance_name) if report.covered_lines: result[curr_path] = None for entry in report.entries: @@ -88,7 +92,7 @@ def _append_covered_group_n_case( self._append_covered_group_n_case(entry, curr_path, result) elif isinstance(entry, TestCaseReport): if entry.covered_lines: - result[(*curr_path, entry.name)] = None + result[(*curr_path, entry.instance_name)] = None @contextmanager diff --git a/testplan/exporters/testing/pdf/renderers/reports.py b/testplan/exporters/testing/pdf/renderers/reports.py index f14974420..0848db625 100644 --- a/testplan/exporters/testing/pdf/renderers/reports.py +++ b/testplan/exporters/testing/pdf/renderers/reports.py @@ -3,25 +3,26 @@ """ import logging from collections import OrderedDict -from typing import Optional, Tuple +from typing import Optional, Tuple, Union from reportlab.lib import colors, styles from reportlab.platypus import Paragraph from testplan.common.exporters.pdf import RowStyle from testplan.common.utils.registry import Registry -from testplan.common.utils.strings import format_description, wrap, split_text +from testplan.common.utils.strings import format_description, split_text, wrap from testplan.report import ( + ReportCategories, Status, - TestReport, - TestGroupReport, TestCaseReport, - ReportCategories, + TestGroupReport, + TestReport, ) from testplan.report.testing.styles import StyleFlag from testplan.testing import tagging + from . import constants as const -from .base import format_duration, RowData, BaseRowRenderer, MetadataMixin +from .base import BaseRowRenderer, MetadataMixin, RowData, format_duration class ReportRendererRegistry(Registry): @@ -260,6 +261,9 @@ def get_header( [ - ][][][] + This method is also used by its subclass, where source will be of type + ``TestCaseReport``. + :param source: Source object for the renderer. :param depth: Depth of the source object on report tree. Used for indentation. :param row_idx: Index of the current table row to be rendered. @@ -417,7 +421,7 @@ def get_header_linestyle(self) -> Tuple[int, colors.HexColor]: def get_header( self, - source: TestCaseReport, + source: TestGroupReport, depth: int, row_idx: int, ) -> RowData: diff --git a/testplan/parser.py b/testplan/parser.py index 020206c21..574a5d552 100644 --- a/testplan/parser.py +++ b/testplan/parser.py @@ -6,9 +6,11 @@ import copy import json import sys +import warnings from typing import Dict, List import schema + from testplan import defaults from testplan.common.utils import logger from testplan.report.testing import ( @@ -488,6 +490,15 @@ def process_args(self, namespace: argparse.Namespace) -> Dict: if args["list"] and not args["test_lister"]: args["test_lister"] = listing.NameLister() + if ( + args["interactive_port"] is not None + and args["tracing_tests"] is not None + ): + warnings.warn( + "Tracing tests feature not available in interactive mode." + ) + args["tracing_tests"] = None + return args diff --git a/testplan/runnable/base.py b/testplan/runnable/base.py index 90c1bba30..c02f3aa13 100644 --- a/testplan/runnable/base.py +++ b/testplan/runnable/base.py @@ -12,6 +12,7 @@ from typing import ( Any, Callable, + Collection, Dict, List, MutableMapping, @@ -19,7 +20,6 @@ Pattern, Tuple, Union, - Collection, ) import pytz @@ -33,11 +33,7 @@ RunnableResult, RunnableStatus, ) -from testplan.common.exporters import ( - BaseExporter, - ExportContext, - run_exporter, -) +from testplan.common.exporters import BaseExporter, ExportContext, run_exporter from testplan.common.remote.remote_service import RemoteService from testplan.common.report import MergeError from testplan.common.utils import logger, strings @@ -56,12 +52,12 @@ from testplan.report.testing.styles import Style from testplan.runnable.interactive import TestRunnerIHandler from testplan.runners.base import Executor +from testplan.runners.pools.base import Pool from testplan.runners.pools.tasks import Task, TaskResult from testplan.runners.pools.tasks.base import is_task_target from testplan.testing import filtering, listing, ordering, tagging from testplan.testing.base import Test, TestResult from testplan.testing.multitest import MultiTest -from testplan.runners.pools.base import Pool def get_exporters(values): @@ -762,7 +758,7 @@ def add( :rtype: ``str`` or ```NoneType`` """ local_runner = self.resources.first() - resource: Union[Executor, str, None] = resource or local_runner + resource: str = resource or local_runner if resource not in self.resources: raise RuntimeError( @@ -1018,12 +1014,11 @@ def _create_result(self): report.category != ReportCategories.TASK_RERUN and self.cfg.merge_scheduled_parts ): - report.uid = report.name # Save the report temporarily and later will merge it - test_rep_lookup.setdefault(report.uid, []).append( + test_rep_lookup.setdefault(report.instance_name, []).append( (test_results[uid].run, report) ) - if report.uid not in test_report.entry_uids: + if report.instance_name not in test_report.entry_uids: # Create a placeholder for merging sibling reports if isinstance(resource_result, TaskResult): # `runnable` must be an instance of MultiTest since @@ -1038,16 +1033,11 @@ def _create_result(self): else: report = report.__class__( - report.name, - uid=report.uid, + report.instance_name, category=report.category, ) else: continue # Wait all sibling reports collected - else: - # If do not want to merge sibling reports, then display - # them with different names. (e.g. `MTest - part(0/3)`) - report.name = report.uid test_report.append(report) step_result = step_result and run is True # boolean or exception diff --git a/testplan/runnable/interactive/base.py b/testplan/runnable/interactive/base.py index 829391d92..57f4a0e17 100644 --- a/testplan/runnable/interactive/base.py +++ b/testplan/runnable/interactive/base.py @@ -5,20 +5,19 @@ import re import threading from concurrent import futures -from typing import Union, Awaitable, Dict, Optional +from typing import Awaitable, Dict, Optional, Union from testplan.common import config, entity from testplan.common.report import Report from testplan.common.utils import networking - -from testplan.runnable.interactive import http, reloader, resource_loader - from testplan.report import ( - TestReport, - TestGroupReport, - Status, + ReportCategories, RuntimeStatus, + Status, + TestGroupReport, + TestReport, ) +from testplan.runnable.interactive import http, reloader, resource_loader def _exclude_assertions_filter(obj: object) -> bool: @@ -567,7 +566,7 @@ def case_filter(obj): if obj.uid == case_uid: return True return obj.uid == suite_uid or ( - obj.category == "parametrization" + obj.category == ReportCategories.PARAMETRIZATION and any(entry.uid == case_uid for entry in obj.entries) ) except Exception: diff --git a/testplan/runners/pools/tasks/base.py b/testplan/runners/pools/tasks/base.py index 9a8ed84be..fe237f0fa 100644 --- a/testplan/runners/pools/tasks/base.py +++ b/testplan/runners/pools/tasks/base.py @@ -5,13 +5,13 @@ import os import warnings from collections import OrderedDict -from typing import Optional, Tuple, Union, Dict, Sequence, Callable +from typing import Callable, Dict, Optional, Sequence, Tuple, Union -from testplan.testing.base import Test, TestResult from testplan.common.serialization import SelectiveSerializable from testplan.common.utils import strings from testplan.common.utils.package import import_tmp_module from testplan.common.utils.path import is_subdir, pwd, rebase_path +from testplan.testing.base import Test, TestResult from testplan.testing.multitest import MultiTest @@ -63,24 +63,21 @@ def __init__( self._kwargs = kwargs or dict() self._uid = uid or strings.uuid4() self._aborted = False - self._max_rerun_limit = ( - self.MAX_RERUN_LIMIT - if rerun > self.MAX_RERUN_LIMIT - else int(rerun) - ) self._assign_for_rerun = 0 self._executors = OrderedDict() self.priority = -weight - if self._max_rerun_limit < 0: + if rerun < 0: raise ValueError("Value of `rerun` cannot be negative.") - elif self._max_rerun_limit > self.MAX_RERUN_LIMIT: + elif rerun > self.MAX_RERUN_LIMIT: warnings.warn( "Value of `rerun` cannot exceed {}".format( self.MAX_RERUN_LIMIT ) ) self._max_rerun_limit = self.MAX_RERUN_LIMIT + else: + self._max_rerun_limit = rerun self._part = part @@ -155,9 +152,9 @@ def reassign_cnt(self) -> int: def reassign_cnt(self, value: int): if value < 0: raise ValueError("Value of `reassign_cnt` cannot be negative") - elif value > self.MAX_RERUN_LIMIT: + elif value > self._max_rerun_limit: raise ValueError( - f"Value of `reassign_cnt` cannot exceed {self.MAX_RERUN_LIMIT}" + f"Value of `reassign_cnt` cannot exceed {self._max_rerun_limit}" ) self._assign_for_rerun = value diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 27930ece3..b7e35cec6 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -3,7 +3,7 @@ import subprocess import sys import warnings -from typing import List, Optional, Dict, Generator +from typing import Dict, Generator, List, Optional from schema import And, Or, Use @@ -189,7 +189,7 @@ def get_filter_levels(self): @property def name(self): - """Instance name. Also uid.""" + """Instance name.""" return self.cfg.name @property @@ -250,9 +250,9 @@ def should_log_test_result(self, depth, test_obj, style): if isinstance(test_obj, TestGroupReport): if depth == 0: return style.display_test, TEST_INST_INDENT - elif test_obj.category == "testsuite": + elif test_obj.category == ReportCategories.TESTSUITE: return style.display_testsuite, SUITE_INDENT - elif test_obj.category == "parametrization": + elif test_obj.category == ReportCategories.PARAMETRIZATION: return False, 0 # DO NOT display else: raise ValueError( diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index 935c68bbb..a6cf59167 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -5,7 +5,7 @@ import functools import itertools import os -from typing import Callable, Optional, Tuple, Dict, List, Generator +from typing import Callable, Dict, Generator, List, Optional, Tuple from schema import And, Or, Use @@ -359,11 +359,7 @@ def setup(self): """ # watch line features depends on configuration from the outside world - if ( - self.cfg.parent is not None - and self.cfg.tracing_tests is not None - and self.cfg.interactive_port is None - ): + if self.cfg.parent is not None and self.cfg.tracing_tests is not None: self.watcher.set_watching_lines(self.cfg.tracing_tests) def get_test_context(self): @@ -427,7 +423,7 @@ def get_test_context(self): testcase_instance = ":".join( [ self.name, - suite.__class__.__name__, + suite.name, testcase.name, ] ) @@ -777,8 +773,9 @@ def _new_test_report(self): :return: A new and empty test report object for this MultiTest. """ return TestGroupReport( - name=self.cfg.name, + name=self.uid(), # part info populated description=self.cfg.description, + instance_name=self.cfg.name, uid=self.uid(), category=ReportCategories.MULTITEST, tags=self.cfg.tags, @@ -794,6 +791,7 @@ def _new_testsuite_report(self, testsuite): return TestGroupReport( name=testsuite.name, description=strings.get_docstring(testsuite.__class__), + instance_name=testsuite.name, uid=testsuite.uid(), category=ReportCategories.TESTSUITE, tags=testsuite.__tags__, @@ -807,6 +805,7 @@ def _new_testcase_report(self, testcase): return TestCaseReport( name=testcase.name, description=strings.get_docstring(testcase), + instance_name=testcase.name, uid=testcase.__name__, tags=testcase.__tags__, ) diff --git a/testplan/testing/multitest/parametrization.py b/testplan/testing/multitest/parametrization.py index fe72373ca..1f7f2d62c 100644 --- a/testplan/testing/multitest/parametrization.py +++ b/testplan/testing/multitest/parametrization.py @@ -6,9 +6,8 @@ import re import warnings -from testplan.common.utils import convert -from testplan.common.utils import interface from testplan.common.utils import callable as callable_utils +from testplan.common.utils import convert, interface from testplan.testing import tagging # Although any string will be processed as normal, it's a good @@ -182,7 +181,7 @@ def _generate_kwarg_list(parameters, args, required_args, default_args): def _generate_func( - function, name, name_func, tag_func, docstring_func, tag_dict, kwargs + function, name, name_func, idx, tag_func, docstring_func, tag_dict, kwargs ): """ Generates a new function using the original function, name generation @@ -205,7 +204,11 @@ def _generated(self, env, result): _generated.__name__ = _parametrization_name_func_wrapper( func_name=function.__name__, kwargs=kwargs ) - _generated.name = name_func(name, kwargs) if name_func else name + # Users request the feature that when `name_func` set to `None`, + # then simply append integer suffixes to the names of testcases + _generated.name = ( + name_func(name, kwargs) if name_func is not None else f"{name} {idx}" + ) if hasattr(function, "__xfail__"): _generated.__xfail__ = function.__xfail__ @@ -271,18 +274,18 @@ def _parametrization_name_func_wrapper(func_name, kwargs): If somehow a 'bad' function name is generated, will just return the original ``func_name`` instead (which will later on be suffixed with an - integer by :py:func:`_ensure_unique_testcase_names`) + integer by :py:func:`_ensure_unique_generated_testcase_names`) """ generated_name = parametrization_name_func(func_name, kwargs) if not PYTHON_VARIABLE_REGEX.match(generated_name): - # Generated method name is not a valid Python attribute name - # e.g. "{func_name}_1", "{func_name}_2" will be used. + # Generated method name is not a valid Python attribute name, + # i.e. "{func_name}__1", "{func_name}__2" will be used. return func_name if len(generated_name) > MAX_METHOD_NAME_LENGTH: # Generated method name is a bit too long. Simple index suffixes - # e.g. "{func_name}_1", "{func_name}_2" will be used. + # e.g. "{func_name}__1", "{func_name}__2" will be used. return func_name return generated_name @@ -408,6 +411,7 @@ def generate_functions( raise ParametrizationError('"parameters" cannot be a empty.') _check_name_func(name_func) + _check_tag_func(tag_func) argspec = callable_utils.getargspec(function) args = argspec.args[3:] # get rid of self, env, result @@ -420,30 +424,24 @@ def generate_functions( parameters, args, required_args, default_args ) - functions = [ - _generate_func( + functions = [] + for idx, kwargs in enumerate(kwarg_list): + func = _generate_func( function=function, name=name, name_func=name_func, + idx=idx, tag_func=tag_func, docstring_func=docstring_func, tag_dict=tag_dict, kwargs=kwargs, ) - for kwargs in kwarg_list - ] - - for idx, func in enumerate(functions): - # Users request the feature that when `name_func` set to `None`, - # then simply append integer suffixes to the names of testcases - if name_func is None: - func.name = "{} {}".format(func.name, idx) - func.summarize = summarize func.summarize_num_passing = num_passing func.summarize_num_failing = num_failing func.summarize_key_combs_limit = key_combs_limit func.execution_group = execution_group func.timeout = timeout + functions.append(func) return functions diff --git a/testplan/testing/multitest/suite.py b/testplan/testing/multitest/suite.py index 134e867a6..1a18c10bd 100644 --- a/testplan/testing/multitest/suite.py +++ b/testplan/testing/multitest/suite.py @@ -345,24 +345,14 @@ def _ensure_unique_generated_testcase_names(names, functions): name = func.__name__ if name in dupe_names or name in valid_names: count = dupe_counter[name] - while True: - func.__name__ = "{}__{}".format(name, count) - dupe_counter[name] += 1 - if ( - func.__name__ not in dupe_names - and func.__name__ not in valid_names - ): - valid_names.add(func.__name__) - break - else: - valid_names.add(name) + func.__name__ = "{}__{}".format(name, count) + dupe_counter[name] += 1 + valid_names.add(func.__name__) # Functions should have different __name__ attributes after the step above - name_counts = collections.Counter( - itertools.chain(names, (func.__name__ for func in functions)) - ) - dupe_names = {k for k, v in name_counts.items() if v > 1} - assert len(dupe_names) == 0 + names = list(itertools.chain(names, (func.__name__ for func in functions))) + if len(set(names)) != len(names): + raise RuntimeError("Duplicate testcase __name__ found.") def _testsuite(klass): diff --git a/testplan/web_ui/testing/src/Nav/NavList.js b/testplan/web_ui/testing/src/Nav/NavList.js index d473952b4..5728af21c 100644 --- a/testplan/web_ui/testing/src/Nav/NavList.js +++ b/testplan/web_ui/testing/src/Nav/NavList.js @@ -19,7 +19,7 @@ const NavList = (props) => { (entry) => ( ": { + "Dummy:DynamicXfailSuiteAlias:test_2 ": { "reason": "unknown non-flaky", "strict": True, }, diff --git a/tests/unit/testplan/common/report/test_base.py b/tests/unit/testplan/common/report/test_base.py index d74d99a4c..6987257f1 100644 --- a/tests/unit/testplan/common/report/test_base.py +++ b/tests/unit/testplan/common/report/test_base.py @@ -1,14 +1,13 @@ -import logging import functools +import logging import re -from unittest import mock import uuid +from unittest import mock import pytest from testplan.common import report from testplan.common.report.log import LOGGER - from testplan.common.utils.testing import disable_log_propagation DummyReport = functools.partial(report.Report, name="dummy") @@ -86,12 +85,12 @@ def test_equality_fail(self, override): assert rep_1 != rep_2 - def test_check_report_id_mismatch(self): + def test_check_report_instance_name_mismatch(self): """Should raise ValueError on failure""" # These will have different ids - rep_1 = DummyReport(uid=1) - rep_2 = DummyReport(uid=2) + rep_1 = DummyReport(instance_name=1) + rep_2 = DummyReport(instance_name=2) with pytest.raises(AttributeError): rep_1._check_report(rep_2) @@ -102,8 +101,8 @@ def test_check_report_type_mismatch(self): class OtherReport(report.Report): pass - rep_1 = DummyReport(uid=5) - rep_2 = OtherReport(name="foo", uid=5) + rep_1 = DummyReport(name="foo") + rep_2 = OtherReport(name="foo") with pytest.raises(TypeError): rep_1._check_report(rep_2)