From e4c71fa934ff00536c697d753018d5568efc1977 Mon Sep 17 00:00:00 2001 From: coutotyler Date: Sat, 1 Feb 2020 14:46:10 -0600 Subject: [PATCH 1/5] Adding test timing functionality --- tests/test_testing.py | 23 ++++++++------ ward/collect.py | 3 +- ward/run.py | 23 ++++++++------ ward/suite.py | 32 ++++--------------- ward/terminal.py | 19 ++++++++--- ward/testing.py | 74 ++++++++++++++++++++++++++++++++++++++----- 6 files changed, 117 insertions(+), 57 deletions(-) diff --git a/tests/test_testing.py b/tests/test_testing.py index d47eed29..3d24ed4f 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -1,16 +1,16 @@ +import sys from collections import defaultdict from pathlib import Path from unittest import mock from unittest.mock import Mock -import sys - -from tests.utilities import testable_test, FORCE_TEST_PATH -from ward import raises, Scope +from ward import Scope, raises from ward.errors import ParameterisationError from ward.fixtures import fixture from ward.models import WardMeta -from ward.testing import Test, test, each, ParamMeta +from ward.testing import ParamMeta, Test, each, test + +from tests.utilities import FORCE_TEST_PATH, testable_test def f(): @@ -179,22 +179,27 @@ def invalid_test(a=each(1, 2), b=each(3, 4, 5)): def i_print_something(): print("out") sys.stderr.write("err") + raise Exception @test("stdout/stderr are captured by default when a test is called") def _(): t = Test(fn=i_print_something, module_name="") t() - assert t.sout.getvalue() == "out\n" - assert t.serr.getvalue() == "err" + assert t.result.captured_stdout == "out\n" + assert t.result.captured_stderr == "err" + # assert t.sout.getvalue() == "out\n" + # assert t.serr.getvalue() == "err" @test("stdout/stderr are not captured when Test.capture_output = False") def _(): t = Test(fn=i_print_something, module_name="", capture_output=False) t() - assert t.sout.getvalue() == "" - assert t.serr.getvalue() == "" + assert t.result.captured_stdout == "" + assert t.result.captured_stderr == "" + # assert t.sout.getvalue() == "" + # assert t.serr.getvalue() == "" @fixture diff --git a/ward/collect.py b/ward/collect.py index a8067b4a..68ab7321 100644 --- a/ward/collect.py +++ b/ward/collect.py @@ -100,7 +100,7 @@ def load_modules(modules: Iterable[pkgutil.ModuleInfo]) -> Generator[Any, None, def get_tests_in_modules( - modules: Iterable, capture_output: bool = True + modules: Iterable, capture_output: bool = True, duration: int = 0 ) -> Generator[Test, None, None]: for mod in modules: mod_name = mod.__name__ @@ -115,6 +115,7 @@ def get_tests_in_modules( marker=meta.marker, description=meta.description or "", capture_output=capture_output, + record_duration=bool(duration), ) diff --git a/ward/run.py b/ward/run.py index e068357f..c7c21718 100644 --- a/ward/run.py +++ b/ward/run.py @@ -1,18 +1,13 @@ +import sys from pathlib import Path from timeit import default_timer from typing import Optional, Tuple import click -import sys from colorama import init - from ward._ward_version import __version__ -from ward.collect import ( - get_info_for_modules, - get_tests_in_modules, - load_modules, - search_generally, -) +from ward.collect import (get_info_for_modules, get_tests_in_modules, + load_modules, search_generally) from ward.config import set_defaults_from_config from ward.rewrite import rewrite_assertions_in_tests from ward.suite import Suite @@ -75,6 +70,13 @@ is_eager=True, help="Look for tests in PATH.", ) +@click.option( + "-d", + "--duration", + type=int, + help="Record and display duration of n longest running tests", + default=0, +) @click.pass_context def run( ctx: click.Context, @@ -86,12 +88,13 @@ def run( order: str, capture_output: bool, config: str, + duration: int, ): start_run = default_timer() paths = [Path(p) for p in path] mod_infos = get_info_for_modules(paths, exclude) modules = list(load_modules(mod_infos)) - unfiltered_tests = get_tests_in_modules(modules, capture_output) + unfiltered_tests = get_tests_in_modules(modules, capture_output, duration) tests = list(search_generally(unfiltered_tests, query=search)) # Rewrite assertions in each test @@ -106,6 +109,8 @@ def run( results = writer.output_all_test_results( test_results, time_to_collect=time_to_collect, fail_limit=fail_limit ) + if duration: + writer.output_longest_durations(results, duration) time_taken = default_timer() - start_run writer.output_test_result_summary(results, time_taken) diff --git a/ward/suite.py b/ward/suite.py index 2477d2cc..15eef6da 100644 --- a/ward/suite.py +++ b/ward/suite.py @@ -34,31 +34,13 @@ def generate_test_runs(self, order="standard") -> Generator[TestResult, None, No generated_tests = test.get_parameterised_instances() num_tests_per_module[test.path] -= 1 for i, generated_test in enumerate(generated_tests): - marker = generated_test.marker.name if generated_test.marker else None - if marker == "SKIP": - yield generated_test.get_result(TestOutcome.SKIP) - continue - - try: - resolved_vals = generated_test.resolve_args(self.cache, iteration=i) - generated_test.format_description(resolved_vals) - generated_test(**resolved_vals) - outcome = ( - TestOutcome.XPASS if marker == "XFAIL" else TestOutcome.PASS - ) - yield generated_test.get_result(outcome) - except FixtureError as e: - yield generated_test.get_result(TestOutcome.FAIL, e) - continue - except Exception as e: - outcome = ( - TestOutcome.XFAIL if marker == "XFAIL" else TestOutcome.FAIL - ) - yield generated_test.get_result(outcome, e) - finally: - self.cache.teardown_fixtures_for_scope( - Scope.Test, scope_key=generated_test.id - ) + resolved_vals = generated_test.resolve_args(self.cache, iteration=i) + generated_test.format_description(resolved_vals) + generated_test(**resolved_vals) + yield generated_test.result + self.cache.teardown_fixtures_for_scope( + Scope.Test, scope_key=generated_test.id + ) if num_tests_per_module[test.path] == 0: self.cache.teardown_fixtures_for_scope( diff --git a/ward/terminal.py b/ward/terminal.py index 579c3cbd..43586aeb 100644 --- a/ward/terminal.py +++ b/ward/terminal.py @@ -1,23 +1,22 @@ import inspect import os import platform +import sys import traceback from dataclasses import dataclass from enum import Enum from pathlib import Path -from textwrap import wrap, indent -from typing import Any, Dict, Generator, List, Optional, Iterable +from textwrap import indent, wrap +from typing import Any, Dict, Generator, Iterable, List, Optional -import sys from colorama import Fore, Style from pygments import highlight from pygments.formatters.terminal import TerminalFormatter from pygments.lexers.python import PythonLexer from termcolor import colored, cprint - from ward._ward_version import __version__ from ward.diff import make_diff -from ward.expect import TestFailure, Comparison +from ward.expect import Comparison, TestFailure from ward.suite import Suite from ward.testing import TestOutcome, TestResult @@ -424,6 +423,16 @@ def output_test_result_summary( print(output) + def output_longest_durations(self, test_results: List[TestResult], num_tests: int): + test_results = sorted( + test_results, key=lambda r: r.test.timer.duration, reverse=True + ) + print(f"{num_tests} Longest Running Tests:") + for result in test_results[:num_tests]: + print( + f"{result.test.timer.duration:.2f} sec - {result.test.description} - {result.test.module_name}:{result.test.line_number}" + ) + def output_captured_stderr(self, test_result: TestResult): if test_result.captured_stderr: captured_stderr_lines = test_result.captured_stderr.split("\n") diff --git a/ward/testing.py b/ward/testing.py index 810d4c67..2372f5d8 100644 --- a/ward/testing.py +++ b/ward/testing.py @@ -2,17 +2,18 @@ import inspect import uuid from collections import defaultdict -from contextlib import closing, redirect_stderr, redirect_stdout +from contextlib import ExitStack, closing, redirect_stderr, redirect_stdout from dataclasses import dataclass, field -from enum import auto, Enum +from enum import Enum, auto from io import StringIO from pathlib import Path +from time import time from types import MappingProxyType -from typing import Callable, Dict, List, Optional, Any, Tuple, Union +from typing import Any, Callable, Dict, List, Optional, Tuple, Union from ward.errors import FixtureError, ParameterisationError from ward.fixtures import Fixture, FixtureCache, ScopeKey -from ward.models import Marker, SkipMarker, XfailMarker, WardMeta, Scope +from ward.models import Marker, Scope, SkipMarker, WardMeta, XfailMarker from ward.util import get_absolute_path @@ -104,12 +105,56 @@ class Test: sout: StringIO = field(default_factory=StringIO) serr: StringIO = field(default_factory=StringIO) ward_meta: WardMeta = field(default_factory=WardMeta) + record_duration: bool = True + timer: Optional["Timer"] = None + result: Optional["TestResult"] = None def __call__(self, *args, **kwargs): - if self.capture_output: - with redirect_stdout(self.sout), redirect_stderr(self.serr): - return self.fn(*args, **kwargs) - return self.fn(*args, **kwargs) + with ExitStack() as stack: + if self.capture_output: + stack.enter_context(redirect_stdout(self.sout)) + stack.enter_context(redirect_stderr(self.serr)) + if self.record_duration: + self.timer = stack.enter_context(Timer()) + + if isinstance(self.marker, SkipMarker): + # TODO: Do sout and serr need to be part of the class? If they weren't we could have + # the ExitStack close them + with closing(self.sout), closing(self.serr): + self.result = TestResult(self, TestOutcome.SKIP) + return + + try: + self.fn(*args, **kwargs) + except FixtureError as e: + outcome = TestOutcome.FAIL + error = e + except Exception as e: + outcome = ( + TestOutcome.XFAIL + if isinstance(self.marker, XfailMarker) + else TestOutcome.FAIL + ) + error = e + else: + outcome = ( + TestOutcome.XPASS + if isinstance(self.marker, XfailMarker) + else TestOutcome.PASS + ) + error = None + + with closing(self.sout), closing(self.serr): + if outcome in (TestOutcome.PASS, TestOutcome.SKIP): + self.result = TestResult(self, outcome) + else: + self.result = TestResult( + self, + outcome, + error, + captured_stdout=self.sout.getvalue(), + captured_stderr=self.serr.getvalue(), + ) @property def name(self) -> str: @@ -409,3 +454,16 @@ class TestResult: message: str = "" captured_stdout: str = "" captured_stderr: str = "" + + +class Timer: + def __init__(self): + self._start_time = None + self.duration = None + + def __enter__(self): + self._start_time = time() + return self + + def __exit__(self, *args): + self.duration = time() - self._start_time From 8c08eb5ed7e29b35b391f1d1c3a1d8cf9b84f04c Mon Sep 17 00:00:00 2001 From: coutotyler Date: Mon, 3 Feb 2020 17:30:16 -0600 Subject: [PATCH 2/5] Changes from review - Timing all tests by default and only displaying slowest runs if requested - Passing duration into output_test_result_summary instead of output_all_test_results - Removing link from Test to TestResult - Defaulting to timeit.default_timer() --- tests/test_testing.py | 37 ++++++++++---------- ward/collect.py | 3 +- ward/run.py | 6 ++-- ward/suite.py | 16 +++++---- ward/terminal.py | 53 ++++++++++++++++++++-------- ward/testing.py | 80 ++++++++++++++++++++++++++----------------- 6 files changed, 119 insertions(+), 76 deletions(-) diff --git a/tests/test_testing.py b/tests/test_testing.py index 3d24ed4f..68947142 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -6,7 +6,7 @@ from ward import Scope, raises from ward.errors import ParameterisationError -from ward.fixtures import fixture +from ward.fixtures import FixtureCache, fixture from ward.models import WardMeta from ward.testing import ParamMeta, Test, each, test @@ -41,6 +41,11 @@ def _(a=x): return Test(fn=_, module_name=mod) +@fixture() +def cache(): + return FixtureCache() + + @test("Test.name should return the name of the function it wraps") def _(anonymous_test=anonymous_test): assert anonymous_test.name == "_" @@ -77,12 +82,12 @@ def _(anonymous_test=anonymous_test): assert not anonymous_test.has_deps -@test("Test.__call__ should delegate to the function it wraps") -def _(): +@test("Test.run should delegate to the function it wraps") +def _(cache: FixtureCache = cache): mock = Mock() - t = Test(fn=mock, module_name=mod) - t(1, 2, key="val") - mock.assert_called_once_with(1, 2, key="val") + t = Test(fn=mock, module_name=mod, args={"key": "val"}) + t.run(cache) + mock.assert_called_once_with(key="val") @test("Test.is_parameterised should return True for parameterised test") @@ -183,23 +188,19 @@ def i_print_something(): @test("stdout/stderr are captured by default when a test is called") -def _(): +def _(cache: FixtureCache = cache): t = Test(fn=i_print_something, module_name="") - t() - assert t.result.captured_stdout == "out\n" - assert t.result.captured_stderr == "err" - # assert t.sout.getvalue() == "out\n" - # assert t.serr.getvalue() == "err" + result = t.run(cache) + assert result.captured_stdout == "out\n" + assert result.captured_stderr == "err" @test("stdout/stderr are not captured when Test.capture_output = False") -def _(): +def _(cache: FixtureCache = cache): t = Test(fn=i_print_something, module_name="", capture_output=False) - t() - assert t.result.captured_stdout == "" - assert t.result.captured_stderr == "" - # assert t.sout.getvalue() == "" - # assert t.serr.getvalue() == "" + result = t.run(cache) + assert result.captured_stdout == "" + assert result.captured_stderr == "" @fixture diff --git a/ward/collect.py b/ward/collect.py index 68ab7321..a8067b4a 100644 --- a/ward/collect.py +++ b/ward/collect.py @@ -100,7 +100,7 @@ def load_modules(modules: Iterable[pkgutil.ModuleInfo]) -> Generator[Any, None, def get_tests_in_modules( - modules: Iterable, capture_output: bool = True, duration: int = 0 + modules: Iterable, capture_output: bool = True ) -> Generator[Test, None, None]: for mod in modules: mod_name = mod.__name__ @@ -115,7 +115,6 @@ def get_tests_in_modules( marker=meta.marker, description=meta.description or "", capture_output=capture_output, - record_duration=bool(duration), ) diff --git a/ward/run.py b/ward/run.py index c7c21718..ab155d64 100644 --- a/ward/run.py +++ b/ward/run.py @@ -94,7 +94,7 @@ def run( paths = [Path(p) for p in path] mod_infos = get_info_for_modules(paths, exclude) modules = list(load_modules(mod_infos)) - unfiltered_tests = get_tests_in_modules(modules, capture_output, duration) + unfiltered_tests = get_tests_in_modules(modules, capture_output) tests = list(search_generally(unfiltered_tests, query=search)) # Rewrite assertions in each test @@ -109,10 +109,8 @@ def run( results = writer.output_all_test_results( test_results, time_to_collect=time_to_collect, fail_limit=fail_limit ) - if duration: - writer.output_longest_durations(results, duration) time_taken = default_timer() - start_run - writer.output_test_result_summary(results, time_taken) + writer.output_test_result_summary(results, time_taken, duration) exit_code = get_exit_code(results) diff --git a/ward/suite.py b/ward/suite.py index 15eef6da..5643c459 100644 --- a/ward/suite.py +++ b/ward/suite.py @@ -4,9 +4,8 @@ from typing import Generator, List from ward import Scope -from ward.errors import FixtureError from ward.fixtures import FixtureCache -from ward.testing import Test, TestOutcome, TestResult +from ward.testing import Test, TestResult @dataclass @@ -26,6 +25,12 @@ def _test_counts_per_module(self): return counts def generate_test_runs(self, order="standard") -> Generator[TestResult, None, None]: + """ + Run tests + + Returns a generator which yields test results + """ + if order == "random": shuffle(self.tests) @@ -33,11 +38,8 @@ def generate_test_runs(self, order="standard") -> Generator[TestResult, None, No for test in self.tests: generated_tests = test.get_parameterised_instances() num_tests_per_module[test.path] -= 1 - for i, generated_test in enumerate(generated_tests): - resolved_vals = generated_test.resolve_args(self.cache, iteration=i) - generated_test.format_description(resolved_vals) - generated_test(**resolved_vals) - yield generated_test.result + for idx, generated_test in enumerate(generated_tests): + yield generated_test.run(self.cache, idx) self.cache.teardown_fixtures_for_scope( Scope.Test, scope_key=generated_test.id ) diff --git a/ward/terminal.py b/ward/terminal.py index 43586aeb..9836b54b 100644 --- a/ward/terminal.py +++ b/ward/terminal.py @@ -39,11 +39,28 @@ def multiline_description(s: str, indent: int, width: int) -> str: return rv -def output_test_result_line(test_result: TestResult): - colour = outcome_to_colour(test_result.outcome) - bg = f"on_{colour}" - padded_outcome = f" {test_result.outcome.name[:4]} " +def format_test_id(test_result: TestResult) -> (str, str): + """ + Format module name, line number, and test case number + """ + + test_id = lightblack( + f"{format_test_location(test_result)}{format_test_case_number(test_result)}:" + ) + + return test_id + + +def format_test_location(test_result: TestResult) -> str: + """ + """ + + return f"{test_result.test.module_name}:{test_result.test.line_number}" + +def format_test_case_number(test_result: TestResult) -> str: + """ + """ # If we're executing a parameterised test param_meta = test_result.test.param_meta if param_meta.group_size > 1: @@ -54,11 +71,16 @@ def output_test_result_line(test_result: TestResult): else: iter_indicator = "" - mod_name = lightblack( - f"{test_result.test.module_name}:" - f"{test_result.test.line_number}" - f"{iter_indicator}:" - ) + return iter_indicator + + +def output_test_result_line(test_result: TestResult): + colour = outcome_to_colour(test_result.outcome) + bg = f"on_{colour}" + padded_outcome = f" {test_result.outcome.name[:4]} " + + iter_indicator = format_test_case_number(test_result) + mod_name = format_test_id(test_result) if ( test_result.outcome == TestOutcome.SKIP or test_result.outcome == TestOutcome.XFAIL @@ -259,7 +281,7 @@ def output_why_test_failed_header(self, test_result: TestResult): raise NotImplementedError() def output_test_result_summary( - self, test_results: List[TestResult], time_taken: float + self, test_results: List[TestResult], time_taken: float, duration: int ): raise NotImplementedError() @@ -384,8 +406,9 @@ def result_checkbox(self, expect): return result_marker def output_test_result_summary( - self, test_results: List[TestResult], time_taken: float + self, test_results: List[TestResult], time_taken: float, duration: int ): + self._output_longest_durations(test_results, duration) outcome_counts = self._get_outcome_counts(test_results) if test_results: chart = self.generate_chart( @@ -423,15 +446,17 @@ def output_test_result_summary( print(output) - def output_longest_durations(self, test_results: List[TestResult], num_tests: int): + def _output_longest_durations(self, test_results: List[TestResult], num_tests: int): test_results = sorted( test_results, key=lambda r: r.test.timer.duration, reverse=True ) - print(f"{num_tests} Longest Running Tests:") + print("Longest Running Tests\n") for result in test_results[:num_tests]: + test_id = format_test_id(result) print( - f"{result.test.timer.duration:.2f} sec - {result.test.description} - {result.test.module_name}:{result.test.line_number}" + f"{result.test.timer.duration:.2f} sec {test_id} {result.test.description} " ) + print() def output_captured_stderr(self, test_result: TestResult): if test_result.captured_stderr: diff --git a/ward/testing.py b/ward/testing.py index 2372f5d8..990298fd 100644 --- a/ward/testing.py +++ b/ward/testing.py @@ -7,7 +7,7 @@ from enum import Enum, auto from io import StringIO from pathlib import Path -from time import time +from timeit import default_timer from types import MappingProxyType from typing import Any, Callable, Dict, List, Optional, Tuple, Union @@ -105,30 +105,36 @@ class Test: sout: StringIO = field(default_factory=StringIO) serr: StringIO = field(default_factory=StringIO) ward_meta: WardMeta = field(default_factory=WardMeta) - record_duration: bool = True timer: Optional["Timer"] = None - result: Optional["TestResult"] = None + args: Optional[Dict[str, Any]] = None + + def run(self, cache: FixtureCache, idx: int = 0) -> "TestResult": + if self.args is None: + raise RuntimeError() - def __call__(self, *args, **kwargs): with ExitStack() as stack: + self.timer = stack.enter_context(Timer()) if self.capture_output: stack.enter_context(redirect_stdout(self.sout)) stack.enter_context(redirect_stderr(self.serr)) - if self.record_duration: - self.timer = stack.enter_context(Timer()) if isinstance(self.marker, SkipMarker): - # TODO: Do sout and serr need to be part of the class? If they weren't we could have + # TODO:onlyanegg: Do sout and serr need to be part of the class? If they weren't we could have # the ExitStack close them with closing(self.sout), closing(self.serr): - self.result = TestResult(self, TestOutcome.SKIP) - return + result = TestResult(self, TestOutcome.SKIP) + return result try: - self.fn(*args, **kwargs) + # TODO:onlyanegg: I don't love this. We're setting up the + # fixture within the testing module, but cleaning it up in the + # suite module. + self.resolve_args(cache, iteration=idx) + self.format_description() + self.fn(**self.args) except FixtureError as e: outcome = TestOutcome.FAIL - error = e + error: Optional[Exception] = e except Exception as e: outcome = ( TestOutcome.XFAIL @@ -146,9 +152,9 @@ def __call__(self, *args, **kwargs): with closing(self.sout), closing(self.serr): if outcome in (TestOutcome.PASS, TestOutcome.SKIP): - self.result = TestResult(self, outcome) + result = TestResult(self, outcome) else: - self.result = TestResult( + result = TestResult( self, outcome, error, @@ -156,6 +162,8 @@ def __call__(self, *args, **kwargs): captured_stderr=self.serr.getvalue(), ) + return result + @property def name(self) -> str: return self.fn.__name__ @@ -211,24 +219,23 @@ def get_parameterised_instances(self) -> List["Test"]: generated_tests = [] for instance_index in range(number_of_instances): - generated_tests.append( - Test( - fn=self.fn, - module_name=self.module_name, - marker=self.marker, - description=self.description, - param_meta=ParamMeta( - instance_index=instance_index, group_size=number_of_instances - ), - capture_output=self.capture_output, - ) + test = Test( + fn=self.fn, + module_name=self.module_name, + marker=self.marker, + description=self.description, + param_meta=ParamMeta( + instance_index=instance_index, group_size=number_of_instances + ), + capture_output=self.capture_output, ) + generated_tests.append(test) return generated_tests def deps(self) -> MappingProxyType: return inspect.signature(self.fn).parameters - def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> Dict[str, Any]: + def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> None: """ Resolve fixtures and return the resultant name -> Fixture dict. If the argument is not a fixture, the raw argument will be used. @@ -237,8 +244,9 @@ def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> Dict[str, Any """ if self.capture_output: with redirect_stdout(self.sout), redirect_stderr(self.serr): - return self._resolve_args(cache, iteration) - return self._resolve_args(cache, iteration) + self.args = self._resolve_args(cache, iteration) + else: + self.args = self._resolve_args(cache, iteration) def _resolve_args(self, cache: FixtureCache, iteration: int) -> Dict[str, Any]: if not self.has_deps: @@ -320,6 +328,13 @@ def _find_number_of_instances(self) -> int: def _resolve_single_arg( self, arg: Callable, cache: FixtureCache ) -> Union[Any, Fixture]: + """ + Get the fixture return value + + If the fixture has been cached, return the value from the cache. + Otherwise, call the fixture function and return the value. + """ + if not hasattr(arg, "ward_meta"): return arg @@ -372,7 +387,7 @@ def _unpack_resolved(self, fixture_dict: Dict[str, Any]) -> Dict[str, Any]: resolved_vals[k] = arg return resolved_vals - def format_description(self, arg_map: Dict[str, Any]) -> str: + def format_description(self) -> str: """ Applies any necessary string formatting to the description, given a dictionary `arg_map` of values that will be injected @@ -381,7 +396,10 @@ def format_description(self, arg_map: Dict[str, Any]) -> str: This method will mutate the Test by updating the description. Returns the newly updated description. """ - format_dict = FormatDict(**arg_map) + if self.args is None: + raise RuntimeError() + + format_dict = FormatDict(**self.args) if not self.description: self.description = "" @@ -462,8 +480,8 @@ def __init__(self): self.duration = None def __enter__(self): - self._start_time = time() + self._start_time = default_timer() return self def __exit__(self, *args): - self.duration = time() - self._start_time + self.duration = default_timer() - self._start_time From b87ad2454d3f8a4cfe730e3ffef850ca0c2fc50b Mon Sep 17 00:00:00 2001 From: coutotyler Date: Mon, 3 Feb 2020 18:35:53 -0600 Subject: [PATCH 3/5] Bug Fixes - Checking for self.args seems useless - Don't pring long running tests if not asked --- ward/terminal.py | 3 ++- ward/testing.py | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/ward/terminal.py b/ward/terminal.py index 9836b54b..8527a487 100644 --- a/ward/terminal.py +++ b/ward/terminal.py @@ -408,7 +408,8 @@ def result_checkbox(self, expect): def output_test_result_summary( self, test_results: List[TestResult], time_taken: float, duration: int ): - self._output_longest_durations(test_results, duration) + if duration: + self._output_longest_durations(test_results, duration) outcome_counts = self._get_outcome_counts(test_results) if test_results: chart = self.generate_chart( diff --git a/ward/testing.py b/ward/testing.py index 990298fd..5b6c08f8 100644 --- a/ward/testing.py +++ b/ward/testing.py @@ -109,8 +109,6 @@ class Test: args: Optional[Dict[str, Any]] = None def run(self, cache: FixtureCache, idx: int = 0) -> "TestResult": - if self.args is None: - raise RuntimeError() with ExitStack() as stack: self.timer = stack.enter_context(Timer()) @@ -396,8 +394,6 @@ def format_description(self) -> str: This method will mutate the Test by updating the description. Returns the newly updated description. """ - if self.args is None: - raise RuntimeError() format_dict = FormatDict(**self.args) if not self.description: From fff2832d39672df8613ae95d234abe17254debb5 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Tue, 4 Feb 2020 01:50:23 +0000 Subject: [PATCH 4/5] Update test_testing.py --- tests/test_testing.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_testing.py b/tests/test_testing.py index 68947142..7c9df003 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -84,10 +84,18 @@ def _(anonymous_test=anonymous_test): @test("Test.run should delegate to the function it wraps") def _(cache: FixtureCache = cache): - mock = Mock() - t = Test(fn=mock, module_name=mod, args={"key": "val"}) + called_with = None + call_kwargs = (), {} + + def func(key="val", **kwargs): + nonlocal called_with, call_kwargs + called_with = key + call_kwargs = kwargs + + t = Test(fn=func, module_name=mod) t.run(cache) - mock.assert_called_once_with(key="val") + assert called_with == "val" + assert call_kwargs == {'kwargs': {}} @test("Test.is_parameterised should return True for parameterised test") From a28c823888550dfd8890c2126c9185c8a1810926 Mon Sep 17 00:00:00 2001 From: coutotyler Date: Wed, 5 Feb 2020 21:36:34 -0600 Subject: [PATCH 5/5] Changes for code review --- tests/test_collect.py | 16 ++++++---------- ward/rewrite.py | 2 +- ward/run.py | 15 +++++++++------ ward/suite.py | 4 ++-- ward/terminal.py | 22 +++++++++------------- ward/testing.py | 21 ++++++++++----------- 6 files changed, 37 insertions(+), 43 deletions(-) diff --git a/tests/test_collect.py b/tests/test_collect.py index d652f643..7853a41d 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -3,18 +3,14 @@ from pathlib import Path from pkgutil import ModuleInfo -from tests.test_util import make_project -from ward import test, fixture, raises -from ward.collect import ( - search_generally, - is_test_module, - get_module_path, - is_excluded_module, - remove_excluded_paths, - handled_within, -) +from ward import fixture, raises, test +from ward.collect import (get_module_path, handled_within, is_excluded_module, + is_test_module, remove_excluded_paths, + search_generally) from ward.testing import Test, each +from tests.test_util import make_project + def named(): assert "fox" == "fox" diff --git a/ward/rewrite.py b/ward/rewrite.py index f2775ff5..3c832e34 100644 --- a/ward/rewrite.py +++ b/ward/rewrite.py @@ -110,7 +110,7 @@ def rewrite_assertion(test: Test) -> Test: # We dedented the code so that it was a valid tree, now re-apply the indent for child in ast.walk(new_tree): if hasattr(child, "col_offset"): - child.col_offset = getattr(child, 'col_offset', 0) + col_offset + child.col_offset = getattr(child, "col_offset", 0) + col_offset # Reconstruct the test function new_mod_code_obj = compile(new_tree, code_obj.co_filename, "exec") diff --git a/ward/run.py b/ward/run.py index ab155d64..eb28d0e7 100644 --- a/ward/run.py +++ b/ward/run.py @@ -6,8 +6,12 @@ import click from colorama import init from ward._ward_version import __version__ -from ward.collect import (get_info_for_modules, get_tests_in_modules, - load_modules, search_generally) +from ward.collect import ( + get_info_for_modules, + get_tests_in_modules, + load_modules, + search_generally, +) from ward.config import set_defaults_from_config from ward.rewrite import rewrite_assertions_in_tests from ward.suite import Suite @@ -71,8 +75,7 @@ help="Look for tests in PATH.", ) @click.option( - "-d", - "--duration", + "--show-slowest", type=int, help="Record and display duration of n longest running tests", default=0, @@ -88,7 +91,7 @@ def run( order: str, capture_output: bool, config: str, - duration: int, + show_slowest: int, ): start_run = default_timer() paths = [Path(p) for p in path] @@ -110,7 +113,7 @@ def run( test_results, time_to_collect=time_to_collect, fail_limit=fail_limit ) time_taken = default_timer() - start_run - writer.output_test_result_summary(results, time_taken, duration) + writer.output_test_result_summary(results, time_taken, show_slowest) exit_code = get_exit_code(results) diff --git a/ward/suite.py b/ward/suite.py index 5643c459..a80de59c 100644 --- a/ward/suite.py +++ b/ward/suite.py @@ -38,8 +38,8 @@ def generate_test_runs(self, order="standard") -> Generator[TestResult, None, No for test in self.tests: generated_tests = test.get_parameterised_instances() num_tests_per_module[test.path] -= 1 - for idx, generated_test in enumerate(generated_tests): - yield generated_test.run(self.cache, idx) + for generated_test in generated_tests: + yield generated_test.run(self.cache) self.cache.teardown_fixtures_for_scope( Scope.Test, scope_key=generated_test.id ) diff --git a/ward/terminal.py b/ward/terminal.py index 8527a487..4b709878 100644 --- a/ward/terminal.py +++ b/ward/terminal.py @@ -52,15 +52,10 @@ def format_test_id(test_result: TestResult) -> (str, str): def format_test_location(test_result: TestResult) -> str: - """ - """ - return f"{test_result.test.module_name}:{test_result.test.line_number}" def format_test_case_number(test_result: TestResult) -> str: - """ - """ # If we're executing a parameterised test param_meta = test_result.test.param_meta if param_meta.group_size > 1: @@ -406,10 +401,10 @@ def result_checkbox(self, expect): return result_marker def output_test_result_summary( - self, test_results: List[TestResult], time_taken: float, duration: int + self, test_results: List[TestResult], time_taken: float, show_slowest: int ): - if duration: - self._output_longest_durations(test_results, duration) + if show_slowest: + self._output_slowest_tests(test_results, show_slowest) outcome_counts = self._get_outcome_counts(test_results) if test_results: chart = self.generate_chart( @@ -447,16 +442,17 @@ def output_test_result_summary( print(output) - def _output_longest_durations(self, test_results: List[TestResult], num_tests: int): + def _output_slowest_tests(self, test_results: List[TestResult], num_tests: int): test_results = sorted( test_results, key=lambda r: r.test.timer.duration, reverse=True ) - print("Longest Running Tests\n") + self.print_divider() + heading = f"{colored('Longest Running Tests:', color='cyan', attrs=['bold'])}\n" + print(indent(heading, INDENT)) for result in test_results[:num_tests]: test_id = format_test_id(result) - print( - f"{result.test.timer.duration:.2f} sec {test_id} {result.test.description} " - ) + message = f"{result.test.timer.duration:.2f} sec {test_id} {result.test.description} " + print(indent(message, DOUBLE_INDENT)) print() def output_captured_stderr(self, test_result: TestResult): diff --git a/ward/testing.py b/ward/testing.py index 5b6c08f8..6589e5b0 100644 --- a/ward/testing.py +++ b/ward/testing.py @@ -106,7 +106,6 @@ class Test: serr: StringIO = field(default_factory=StringIO) ward_meta: WardMeta = field(default_factory=WardMeta) timer: Optional["Timer"] = None - args: Optional[Dict[str, Any]] = None def run(self, cache: FixtureCache, idx: int = 0) -> "TestResult": @@ -117,8 +116,6 @@ def run(self, cache: FixtureCache, idx: int = 0) -> "TestResult": stack.enter_context(redirect_stderr(self.serr)) if isinstance(self.marker, SkipMarker): - # TODO:onlyanegg: Do sout and serr need to be part of the class? If they weren't we could have - # the ExitStack close them with closing(self.sout), closing(self.serr): result = TestResult(self, TestOutcome.SKIP) return result @@ -127,9 +124,11 @@ def run(self, cache: FixtureCache, idx: int = 0) -> "TestResult": # TODO:onlyanegg: I don't love this. We're setting up the # fixture within the testing module, but cleaning it up in the # suite module. - self.resolve_args(cache, iteration=idx) - self.format_description() - self.fn(**self.args) + resolved_args = self.resolve_args( + cache, iteration=self.param_meta.instance_index + ) + self.format_description(resolved_args) + self.fn(**resolved_args) except FixtureError as e: outcome = TestOutcome.FAIL error: Optional[Exception] = e @@ -233,7 +232,7 @@ def get_parameterised_instances(self) -> List["Test"]: def deps(self) -> MappingProxyType: return inspect.signature(self.fn).parameters - def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> None: + def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> Dict[str, Any]: """ Resolve fixtures and return the resultant name -> Fixture dict. If the argument is not a fixture, the raw argument will be used. @@ -242,9 +241,9 @@ def resolve_args(self, cache: FixtureCache, iteration: int = 0) -> None: """ if self.capture_output: with redirect_stdout(self.sout), redirect_stderr(self.serr): - self.args = self._resolve_args(cache, iteration) + return self._resolve_args(cache, iteration) else: - self.args = self._resolve_args(cache, iteration) + return self._resolve_args(cache, iteration) def _resolve_args(self, cache: FixtureCache, iteration: int) -> Dict[str, Any]: if not self.has_deps: @@ -385,7 +384,7 @@ def _unpack_resolved(self, fixture_dict: Dict[str, Any]) -> Dict[str, Any]: resolved_vals[k] = arg return resolved_vals - def format_description(self) -> str: + def format_description(self, args: Dict[str, Any]) -> str: """ Applies any necessary string formatting to the description, given a dictionary `arg_map` of values that will be injected @@ -395,7 +394,7 @@ def format_description(self) -> str: Returns the newly updated description. """ - format_dict = FormatDict(**self.args) + format_dict = FormatDict(**args) if not self.description: self.description = ""