From a824cd5a316f6001d7ff2c18ca44ad62650d511c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 16:06:29 -0400 Subject: [PATCH 01/10] A lot of cleanups centered around TrialRunner --- src/twisted/trial/runner.py | 188 +++++++++++++------------- src/twisted/trial/test/test_runner.py | 75 ++-------- src/twisted/trial/util.py | 13 +- 3 files changed, 114 insertions(+), 162 deletions(-) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index 47167718b4d..932e1845925 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -32,13 +32,16 @@ import inspect import os import sys -import time import types import warnings +from contextlib import contextmanager from importlib.machinery import SourceFileLoader +from typing import IO, Any, Generator, Optional from zope.interface import implementer +from attrs import define + from twisted.internet import defer from twisted.python import failure, filepath, log, modules, reflect from twisted.trial import unittest, util @@ -49,9 +52,23 @@ # These are imported so that they remain in the public API for t.trial.runner from twisted.trial.unittest import TestSuite +from . import itrial pyunit = __import__("unittest") +from typing import Callable, Protocol + +from typing_extensions import ParamSpec + +_P = ParamSpec("_P") + + +class _Debugger(Protocol): + def runcall( + self, f: Callable[_P, object], *args: _P.args, **kwargs: _P.kwargs + ) -> object: + ... + def isPackage(module): """Given an object return True if the object looks like a package""" @@ -765,6 +782,50 @@ def _qualNameWalker(qualName): yield (".".join(qualParts[:-index]), qualParts[-index:]) +@contextmanager +def _testDirectory(workingDirectory: str) -> Generator[None, None, None]: + """ + A context manager which obtains a lock on a trial working directory + and enters (L{os.chdir}) it and then reverses these things. + + @param workingDirectory: A pattern for the basename of the working + directory to acquire. + """ + currentDir = os.getcwd() + base = filepath.FilePath(workingDirectory) + testdir, testDirLock = util._unusedTestDirectory(base) + os.chdir(testdir.path) + + yield + + os.chdir(currentDir) + testDirLock.unlock() + + +@contextmanager +def _logFile(logfile: str) -> Generator[None, None, None]: + """ + A context manager which adds a log observer and then removes it. + + @param logfile: C{"-"} f or stdout logging, otherwise the path to a log + file to which to write. + """ + if logfile == "-": + logFile = sys.stdout + else: + logFile = util.openTestLog(filepath.FilePath(logfile)) + + logFileObserver = log.FileLogObserver(logFile) + observerFunction = logFileObserver.emit + log.startLoggingWithObserver(observerFunction, 0) + + yield + + log.removeObserver(observerFunction) + logFile.close() + + +@define class TrialRunner: """ A specialised runner that the trial front end uses. @@ -773,21 +834,24 @@ class TrialRunner: DEBUG = "debug" DRY_RUN = "dry-run" - def _setUpTestdir(self): - self._tearDownLogFile() - currentDir = os.getcwd() - base = filepath.FilePath(self.workingDirectory) - testdir, self._testDirLock = util._unusedTestDirectory(base) - os.chdir(testdir.path) - return currentDir + reporterFactory: Callable[[IO[str], str, bool, log.LogPublisher], itrial.IReporter] + mode: Optional[str] = None + logfile: str = "test.log" + stream: IO[str] = sys.stdout + profile: bool = False + _tracebackFormat: str = "default" + _realTimeErrors: bool = False + uncleanWarnings: bool = False + workingDirectory: str = "_trial_temp" + _forceGarbageCollection: bool = False + debugger: Optional[_Debugger] = None + _exitFirst: bool = False - def _tearDownTestdir(self, oldDir): - os.chdir(oldDir) - self._testDirLock.unlock() + _result: Optional[Any] = None - _log = log + _log: log.LogPublisher = log # type: ignore[assignment] - def _makeResult(self): + def _makeResult(self) -> itrial.IReporter: reporter = self.reporterFactory( self.stream, self.tbformat, self.rterrors, self._log ) @@ -797,64 +861,28 @@ def _makeResult(self): reporter = UncleanWarningsReporterWrapper(reporter) return reporter - def __init__( - self, - reporterFactory, - mode=None, - logfile="test.log", - stream=sys.stdout, - profile=False, - tracebackFormat="default", - realTimeErrors=False, - uncleanWarnings=False, - workingDirectory=None, - forceGarbageCollection=False, - debugger=None, - exitFirst=False, - ): - self.reporterFactory = reporterFactory - self.logfile = logfile - self.mode = mode - self.stream = stream - self.tbformat = tracebackFormat - self.rterrors = realTimeErrors - self.uncleanWarnings = uncleanWarnings - self._result = None - self.workingDirectory = workingDirectory or "_trial_temp" - self._logFileObserver = None - self._logFileObject = None - self._forceGarbageCollection = forceGarbageCollection - self.debugger = debugger - self._exitFirst = exitFirst - if profile: - self.run = util.profiled(self.run, "profile.data") - - def _tearDownLogFile(self): - if self._logFileObserver is not None: - log.removeObserver(self._logFileObserver.emit) - self._logFileObserver = None - if self._logFileObject is not None: - self._logFileObject.close() - self._logFileObject = None - - def _setUpLogFile(self) -> None: - self._tearDownLogFile() - if self.logfile == "-": - logFile = sys.stdout - else: - logFile = util.openTestLog(filepath.FilePath(self.logfile)) - self._logFileObject = logFile - self._logFileObserver = log.FileLogObserver(logFile) - log.startLoggingWithObserver(self._logFileObserver.emit, 0) + @property + def tbformat(self) -> str: + return self._tracebackFormat - def run(self, test): + @property + def rterrors(self) -> bool: + return self._realTimeErrors + + def run(self, test: ITestCase) -> itrial.IReporter: """ Run the test or suite and return a result object. """ test = unittest.decorate(test, ITestCase) - return self._runWithoutDecoration(test, self._forceGarbageCollection) + if self.profile: + run = util.profiled(self._runWithoutDecoration, "profile.data") + else: + run = self._runWithoutDecoration + return run(test, self._forceGarbageCollection) - def _runWithoutDecoration(self, test, forceGarbageCollection=False): + def _runWithoutDecoration( + self, test: ITestCase, forceGarbageCollection: bool = False + ) -> itrial.IReporter: """ Private helper that runs the given test but doesn't decorate it. """ @@ -863,7 +891,6 @@ def _runWithoutDecoration(self, test, forceGarbageCollection=False): # This should move out of the runner and be presumed to be # present suite = TrialSuite([test], forceGarbageCollection) - startTime = time.time() if self.mode == self.DRY_RUN: for single in _iterateTests(suite): result.startTest(single) @@ -871,36 +898,15 @@ def _runWithoutDecoration(self, test, forceGarbageCollection=False): result.stopTest(single) else: if self.mode == self.DEBUG: + assert self.debugger is not None run = lambda: self.debugger.runcall(suite.run, result) else: run = lambda: suite.run(result) - oldDir = self._setUpTestdir() - try: - self._setUpLogFile() + with _testDirectory(self.workingDirectory), _logFile(self.logfile): run() - finally: - self._tearDownLogFile() - self._tearDownTestdir(oldDir) - - endTime = time.time() - done = getattr(result, "done", None) - if done is None: - warnings.warn( - "%s should implement done() but doesn't. Falling back to " - "printErrors() and friends." % reflect.qual(result.__class__), - category=DeprecationWarning, - stacklevel=3, - ) - result.printErrors() - result.writeln(result.separator) - result.writeln( - "Ran %d tests in %.3fs", result.testsRun, endTime - startTime - ) - result.write("\n") - result.printSummary() - else: - result.done() + + result.done() return result def runUntilFailure(self, test): diff --git a/src/twisted/trial/test/test_runner.py b/src/twisted/trial/test/test_runner.py index 1c800b55d98..60174f93ded 100644 --- a/src/twisted/trial/test/test_runner.py +++ b/src/twisted/trial/test/test_runner.py @@ -123,9 +123,6 @@ class TrialRunnerTestsMixin: Mixin defining tests for L{runner.TrialRunner}. """ - def tearDown(self): - self.runner._tearDownLogFile() - def test_empty(self): """ Empty test method, used by the other tests. @@ -148,34 +145,15 @@ def test_logFileAlwaysActive(self): """ Test that a new file is opened on each run. """ - oldSetUpLogFile = self.runner._setUpLogFile - l = [] - - def setUpLogFile(): - oldSetUpLogFile() - l.append(self.runner._logFileObserver) - - self.runner._setUpLogFile = setUpLogFile - self.runner.run(self.test) - self.runner.run(self.test) - self.assertEqual(len(l), 2) - self.assertFalse(l[0] is l[1], "Should have created a new file observer") - - def test_logFileGetsClosed(self): - """ - Test that file created is closed during the run. - """ - oldSetUpLogFile = self.runner._setUpLogFile - l = [] + logPath = FilePath(self.runner.workingDirectory).child(self.runner.logfile) - def setUpLogFile(): - oldSetUpLogFile() - l.append(self.runner._logFileObject) - - self.runner._setUpLogFile = setUpLogFile - self.runner.run(self.test) - self.assertEqual(len(l), 1) - self.assertTrue(l[0].closed) + for i in range(2): + self.runner.run(self.test) + logPath.restat() + self.assertTrue(logPath.exists()) + # We can demonstrate it is re-opened on the next iteration by + # deleting the current version. + logPath.remove() class TrialRunnerTests(TrialRunnerTestsMixin, unittest.SynchronousTestCase): @@ -343,17 +321,6 @@ def parseOptions(self, args): def getRunner(self): r = trial._makeRunner(self.config) r.stream = StringIO() - # XXX The runner should always take care of cleaning this up itself. - # It's not clear why this is necessary. The runner always tears down - # its log file. - self.addCleanup(r._tearDownLogFile) - # XXX The runner should always take care of cleaning this up itself as - # well. It's necessary because TrialRunner._setUpTestdir might raise - # an exception preventing Reporter.done from being run, leaving the - # observer added by Reporter.__init__ still present in the system. - # Something better needs to happen inside - # TrialRunner._runWithoutDecoration to remove the need for this cludge. - r._log = log.LogPublisher() return r def test_runner_can_get_reporter(self): @@ -1036,32 +1003,6 @@ def write(self, *args): def writeln(self, *args): pass - def test_reporterDeprecations(self): - """ - The runner emits a warning if it is using a result that doesn't - implement 'done'. - """ - trialRunner = runner.TrialRunner(None) - result = self.FakeReporter() - trialRunner._makeResult = lambda: result - - def f(): - # We have to use a pyunit test, otherwise we'll get deprecation - # warnings about using iterate() in a test. - trialRunner.run(pyunit.TestCase("id")) - - f() - warnings = self.flushWarnings([self.test_reporterDeprecations]) - - self.assertEqual(warnings[0]["category"], DeprecationWarning) - self.assertEqual( - warnings[0]["message"], - "%s should implement done() but doesn't. Falling back to " - "printErrors() and friends." % reflect.qual(result.__class__), - ) - self.assertTrue(__file__.startswith(warnings[0]["filename"])) - self.assertEqual(len(warnings), 1) - class QualifiedNameWalkerTests(unittest.SynchronousTestCase): """ diff --git a/src/twisted/trial/util.py b/src/twisted/trial/util.py index 3f064801eaa..53ff516e7c8 100644 --- a/src/twisted/trial/util.py +++ b/src/twisted/trial/util.py @@ -17,9 +17,10 @@ @var DEFAULT_TIMEOUT_DURATION: The default timeout which will be applied to asynchronous (ie, Deferred-returning) test methods, in seconds. """ - from random import randrange -from typing import TextIO +from typing import Callable, TextIO, TypeVar + +from typing_extensions import ParamSpec from twisted.internet import interfaces, utils from twisted.python.failure import Failure @@ -240,8 +241,12 @@ def suppress(action="ignore", **kwarg): # This should be deleted, and replaced with twisted.application's code; see # #6016: -def profiled(f, outputFile): - def _(*args, **kwargs): +_P = ParamSpec("_P") +_T = TypeVar("_T") + + +def profiled(f: Callable[_P, _T], outputFile: str) -> Callable[_P, _T]: + def _(*args: _P.args, **kwargs: _P.kwargs) -> _T: import profile prof = profile.Profile() From 5792b6be3ebbf7baeeb18b4f9e2211589e6e3312 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 16:18:25 -0400 Subject: [PATCH 02/10] Annotate a few other pieces where script and runner interact --- src/twisted/trial/_asyncrunner.py | 9 ++++++--- src/twisted/trial/runner.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/twisted/trial/_asyncrunner.py b/src/twisted/trial/_asyncrunner.py index 7978a44e623..54e30969714 100644 --- a/src/twisted/trial/_asyncrunner.py +++ b/src/twisted/trial/_asyncrunner.py @@ -10,6 +10,7 @@ import doctest import gc import unittest as pyunit +from typing import Iterator, Union from zope.interface import implementer @@ -160,14 +161,16 @@ def run(self, result): components.registerAdapter(_BrokenIDTestCaseAdapter, _docTestCase, itrial.ITestCase) -def _iterateTests(testSuiteOrCase): +def _iterateTests( + testSuiteOrCase: Union[pyunit.TestCase, pyunit.TestSuite] +) -> Iterator[itrial.ITestCase]: """ Iterate through all of the test cases in C{testSuiteOrCase}. """ try: - suite = iter(testSuiteOrCase) + suite = iter(testSuiteOrCase) # type: ignore[arg-type] except TypeError: - yield testSuiteOrCase + yield testSuiteOrCase # type: ignore[misc] else: for test in suite: yield from _iterateTests(test) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index 932e1845925..c57d0308765 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -909,7 +909,7 @@ def _runWithoutDecoration( result.done() return result - def runUntilFailure(self, test): + def runUntilFailure(self, test: ITestCase) -> itrial.IReporter: """ Repeatedly run C{test} until it fails. """ From 36694e5005739d7448e9e63ead7d4651fd829c18 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 20:55:49 -0400 Subject: [PATCH 03/10] some further improvements, mostly type annotations --- src/twisted/python/modules.py | 7 +-- src/twisted/python/reflect.py | 2 +- src/twisted/scripts/trial.py | 33 +++++------ src/twisted/trial/_asyncrunner.py | 4 +- src/twisted/trial/_dist/disttrial.py | 44 +++++++------- src/twisted/trial/runner.py | 87 ++++++++++++++++++---------- 6 files changed, 103 insertions(+), 74 deletions(-) diff --git a/src/twisted/python/modules.py b/src/twisted/python/modules.py index 2eea15d0c84..aea6a6ed828 100644 --- a/src/twisted/python/modules.py +++ b/src/twisted/python/modules.py @@ -64,7 +64,6 @@ # let's try to keep path imports to a minimum... from os.path import dirname, split as splitpath -from typing import cast from zope.interface import Interface, implementer @@ -262,7 +261,7 @@ def __init__(self, name, onObject, loaded, pythonValue): @param loaded: always True, for now @param pythonValue: the value of the attribute we're pointing to. """ - self.name = name + self.name: str = name self.onObject = onObject self._loaded = loaded self.pythonValue = pythonValue @@ -318,7 +317,7 @@ def __init__(self, name, filePath, pathEntry): """ _name = nativeString(name) assert not _name.endswith(".__init__") - self.name = _name + self.name: str = _name self.filePath = filePath self.parentPath = filePath.parent() self.pathEntry = pathEntry @@ -397,7 +396,7 @@ def __eq__(self, other: object) -> bool: PythonModules with the same name are equal. """ if isinstance(other, PythonModule): - return cast(bool, other.name == self.name) + return other.name == self.name return NotImplemented def walkModules(self, importPackages=False): diff --git a/src/twisted/python/reflect.py b/src/twisted/python/reflect.py index c442704b7d7..9991b4eb63a 100644 --- a/src/twisted/python/reflect.py +++ b/src/twisted/python/reflect.py @@ -348,7 +348,7 @@ def filenameToModuleName(fn): return modName -def qual(clazz): +def qual(clazz: Type[object]) -> str: """ Return full import path of a class. """ diff --git a/src/twisted/scripts/trial.py b/src/twisted/scripts/trial.py index 08904bb4293..38e0c971d51 100644 --- a/src/twisted/scripts/trial.py +++ b/src/twisted/scripts/trial.py @@ -10,7 +10,9 @@ import random import sys import time +import trace import warnings +from typing import NoReturn, Optional, Type from twisted import plugin from twisted.application import app @@ -19,6 +21,8 @@ from twisted.python.filepath import FilePath from twisted.python.reflect import namedModule from twisted.trial import itrial, reporter, runner +from twisted.trial._dist.disttrial import DistTrialRunner +from twisted.trial.unittest import TestSuite # Yea, this is stupid. Leave it for command-line compatibility for a # while, though. @@ -232,7 +236,7 @@ class _BasicOptions: ) fallbackReporter = reporter.TreeReporter - tracer = None + tracer: Optional[trace.Trace] = None def __init__(self): self["tests"] = [] @@ -275,8 +279,6 @@ def opt_coverage(self): Generate coverage information in the coverage file in the directory specified by the temp-directory option. """ - import trace - self.tracer = trace.Trace(count=1, trace=0) sys.settrace(self.tracer.globaltrace) self["coverage"] = True @@ -476,7 +478,6 @@ class Options(_BasicOptions, usage.Options, app.ReactorSelectionMixin): fallbackReporter = reporter.TreeReporter extra = None - tracer = None def opt_jobs(self, number): """ @@ -523,7 +524,7 @@ def postOptions(self): failure.DO_POST_MORTEM = False -def _initialDebugSetup(config): +def _initialDebugSetup(config: Options) -> None: # do this part of debug setup first for easy debugging of import failures if config["debug"]: failure.startDebugMode() @@ -531,13 +532,13 @@ def _initialDebugSetup(config): defer.setDebugging(True) -def _getSuite(config): +def _getSuite(config: Options) -> TestSuite: loader = _getLoader(config) recurse = not config["no-recurse"] return loader.loadByNames(config["tests"], recurse=recurse) -def _getLoader(config): +def _getLoader(config: Options) -> runner.TestLoader: loader = runner.TestLoader() if config["random"]: randomer = random.Random() @@ -584,16 +585,14 @@ class _DebuggerNotFound(Exception): """ -def _makeRunner(config): +def _makeRunner(config: Options) -> runner._Runner: """ Return a trial runner class set up with the parameters extracted from C{config}. @return: A trial runner instance. - @rtype: L{runner.TrialRunner} or C{DistTrialRunner} depending on the - configuration. """ - cls = runner.TrialRunner + cls: Type[runner._Runner] = runner.TrialRunner args = { "reporterFactory": config["reporter"], "tracebackFormat": config["tbformat"], @@ -606,8 +605,6 @@ def _makeRunner(config): if config["dry-run"]: args["mode"] = runner.TrialRunner.DRY_RUN elif config["jobs"]: - from twisted.trial._dist.disttrial import DistTrialRunner - cls = DistTrialRunner args["maxWorkers"] = config["jobs"] args["workerArguments"] = config._getWorkerArguments() @@ -632,7 +629,7 @@ def _makeRunner(config): return cls(**args) -def run(): +def run() -> NoReturn: if len(sys.argv) == 1: sys.argv.append("--help") config = Options() @@ -649,13 +646,13 @@ def run(): suite = _getSuite(config) if config["until-failure"]: - test_result = trialRunner.runUntilFailure(suite) + testResult = trialRunner.runUntilFailure(suite) else: - test_result = trialRunner.run(suite) + testResult = trialRunner.run(suite) if config.tracer: sys.settrace(None) results = config.tracer.results() results.write_results( - show_missing=1, summary=False, coverdir=config.coverdir().path + show_missing=True, summary=False, coverdir=config.coverdir().path ) - sys.exit(not test_result.wasSuccessful()) + sys.exit(not testResult.wasSuccessful()) diff --git a/src/twisted/trial/_asyncrunner.py b/src/twisted/trial/_asyncrunner.py index 54e30969714..12a8342ffae 100644 --- a/src/twisted/trial/_asyncrunner.py +++ b/src/twisted/trial/_asyncrunner.py @@ -168,9 +168,9 @@ def _iterateTests( Iterate through all of the test cases in C{testSuiteOrCase}. """ try: - suite = iter(testSuiteOrCase) # type: ignore[arg-type] + suite = iter(testSuiteOrCase) # type: ignore[arg-type] except TypeError: - yield testSuiteOrCase # type: ignore[misc] + yield testSuiteOrCase # type: ignore[misc] else: for test in suite: yield from _iterateTests(test) diff --git a/src/twisted/trial/_dist/disttrial.py b/src/twisted/trial/_dist/disttrial.py index 9d831c0c085..a57a4bf37b5 100644 --- a/src/twisted/trial/_dist/disttrial.py +++ b/src/twisted/trial/_dist/disttrial.py @@ -14,7 +14,7 @@ from functools import partial from os.path import isabs from typing import Awaitable, Callable, Iterable, List, Sequence, TextIO, Union, cast -from unittest import TestResult, TestSuite +from unittest import TestCase, TestSuite from attrs import define, field, frozen from attrs.converters import default_if_none @@ -287,7 +287,7 @@ class DistTrialRunner: ``False`` to run through the whole suite and report all of the results at the end. - @ivar _stream: stream which the reporter will use. + @ivar stream: stream which the reporter will use. @ivar _reporterFactory: the reporter class to be used. """ @@ -307,7 +307,8 @@ class DistTrialRunner: converter=default_if_none(factory=_defaultReactor), # type: ignore [misc] ) # mypy doesn't understand the converter - _stream: TextIO = field(default=None, converter=default_if_none(sys.stdout)) # type: ignore [misc] + stream: TextIO = field(default=None, converter=default_if_none(sys.stdout)) # type: ignore [misc] + _tracebackFormat: str = "default" _realTimeErrors: bool = False _uncleanWarnings: bool = False @@ -320,7 +321,7 @@ def _makeResult(self) -> DistReporter: Make reporter factory, and wrap it with a L{DistReporter}. """ reporter = self._reporterFactory( - self._stream, self._tracebackFormat, realtime=self._realTimeErrors + self.stream, self._tracebackFormat, realtime=self._realTimeErrors ) if self._uncleanWarnings: reporter = UncleanWarningsReporterWrapper(reporter) @@ -365,13 +366,13 @@ async def task(case): async def runAsync( self, - suite: TestSuite, + suite: Union[TestCase, TestSuite], untilFailure: bool = False, ) -> DistReporter: """ Spawn local worker processes and load tests. After that, run them. - @param suite: A tests suite to be run. + @param suite: A test or suite to be run. @param untilFailure: If C{True}, continue to run the tests until they fail. @@ -396,7 +397,7 @@ async def runAsync( # Announce that we're beginning. countTestCases result is preferred # (over len(testCases)) because testCases may contain synthetic cases # for error reporting purposes. - self._stream.write(f"Running {suite.countTestCases()} tests.\n") + self.stream.write(f"Running {suite.countTestCases()} tests.\n") # Start the worker pool. startedPool = await poolStarter.start(self._reactor) @@ -410,7 +411,7 @@ async def runAndReport(n: int) -> DistReporter: if untilFailure: # If and only if we're running the suite more than once, # provide a report about which run this is. - self._stream.write(f"Test Pass {n + 1}\n") + self.stream.write(f"Test Pass {n + 1}\n") result = self._makeResult() @@ -439,19 +440,14 @@ async def runAndReport(n: int) -> DistReporter: # Shut down the worker pool. await startedPool.join() - def run(self, suite: TestSuite, untilFailure: bool = False) -> TestResult: - """ - Run a reactor and a test suite. - - @param suite: The test suite to run. - """ + def _run(self, test: Union[TestCase, TestSuite], untilFailure: bool) -> IReporter: result: Union[Failure, DistReporter] def capture(r): nonlocal result result = r - d = Deferred.fromCoroutine(self.runAsync(suite, untilFailure)) + d = Deferred.fromCoroutine(self.runAsync(test, untilFailure)) d.addBoth(capture) d.addBoth(lambda ignored: self._reactor.stop()) self._reactor.run() @@ -464,14 +460,22 @@ def capture(r): # certainly a DistReporter at this point. assert isinstance(result, DistReporter) - # Unwrap the DistReporter to give the caller some regular TestResult + # Unwrap the DistReporter to give the caller some regular IReporter # object. DistReporter isn't type annotated correctly so fix it here. - return cast(TestResult, result.original) + return cast(IReporter, result.original) + + def run(self, test: Union[TestCase, TestSuite]) -> IReporter: + """ + Run a reactor and a test suite. + + @param test: The test or suite to run. + """ + return self._run(test, untilFailure=False) - def runUntilFailure(self, suite): + def runUntilFailure(self, test: Union[TestCase, TestSuite]) -> IReporter: """ Run the tests with local worker processes until they fail. - @param suite: A tests suite to be run. + @param test: The test or suite to run. """ - return self.run(suite, untilFailure=True) + return self._run(test, untilFailure=True) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index c57d0308765..aa94f9e1db6 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -33,14 +33,26 @@ import os import sys import types +import unittest as pyunit import warnings from contextlib import contextmanager from importlib.machinery import SourceFileLoader -from typing import IO, Any, Generator, Optional +from typing import ( + Any, + Callable, + Generator, + List, + Optional, + Protocol, + TextIO, + Type, + Union, +) from zope.interface import implementer from attrs import define +from typing_extensions import ParamSpec, TypeAlias, TypeGuard from twisted.internet import defer from twisted.python import failure, filepath, log, modules, reflect @@ -54,12 +66,6 @@ from twisted.trial.unittest import TestSuite from . import itrial -pyunit = __import__("unittest") - -from typing import Callable, Protocol - -from typing_extensions import ParamSpec - _P = ParamSpec("_P") @@ -275,26 +281,31 @@ def run(self, result): self._bail() -def name(thing): +_Loadable: TypeAlias = Union[ + modules.PythonAttribute, + modules.PythonModule, + pyunit.TestCase, + Type[pyunit.TestCase], +] + + +def name(thing: _Loadable) -> str: """ @param thing: an object from modules (instance of PythonModule, PythonAttribute), a TestCase subclass, or an instance of a TestCase. """ - if isTestCase(thing): + if isinstance(thing, pyunit.TestCase): + return thing.id() + elif isinstance(thing, (modules.PythonAttribute, modules.PythonModule)): + return thing.name + elif isTestCase(thing): # TestCase subclass - theName = reflect.qual(thing) + return reflect.qual(thing) else: - # thing from trial, or thing from modules. - # this monstrosity exists so that modules' objects do not have to - # implement id(). -jml - try: - theName = thing.id() - except AttributeError: - theName = thing.name - return theName + assert False -def isTestCase(obj): +def isTestCase(obj: type) -> TypeGuard[Type[pyunit.TestCase]]: """ @return: C{True} if C{obj} is a class that contains test cases, C{False} otherwise. Used to find all the tests in a module. @@ -381,6 +392,7 @@ def run(self, result): result.stopTest(self) +@define class TestLoader: """ I find tests inside function, modules, files -- whatever -- then return @@ -405,10 +417,8 @@ class TestLoader: methodPrefix = "test" modulePrefix = "test_" - def __init__(self): - self.suiteFactory = TestSuite - self.sorter = name - self._importErrors = [] + suiteFactory: Type[TestSuite] = TestSuite + sorter: Callable[[_Loadable], object] = name def sort(self, xs): """ @@ -708,7 +718,7 @@ def loadByName(self, name, recurse=False): loadTestsFromName = loadByName - def loadByNames(self, names, recurse=False): + def loadByNames(self, names: List[str], recurse: bool = False) -> TestSuite: """ Load some tests by a list of names. @@ -825,6 +835,18 @@ def _logFile(logfile: str) -> Generator[None, None, None]: logFile.close() +class _Runner(Protocol): + stream: TextIO + + def run(self, test: Union[pyunit.TestCase, pyunit.TestSuite]) -> itrial.IReporter: + ... + + def runUntilFailure( + self, test: Union[pyunit.TestCase, pyunit.TestSuite] + ) -> itrial.IReporter: + ... + + @define class TrialRunner: """ @@ -834,10 +856,10 @@ class TrialRunner: DEBUG = "debug" DRY_RUN = "dry-run" - reporterFactory: Callable[[IO[str], str, bool, log.LogPublisher], itrial.IReporter] + reporterFactory: Callable[[TextIO, str, bool, log.LogPublisher], itrial.IReporter] mode: Optional[str] = None logfile: str = "test.log" - stream: IO[str] = sys.stdout + stream: TextIO = sys.stdout profile: bool = False _tracebackFormat: str = "default" _realTimeErrors: bool = False @@ -869,7 +891,7 @@ def tbformat(self) -> str: def rterrors(self) -> bool: return self._realTimeErrors - def run(self, test: ITestCase) -> itrial.IReporter: + def run(self, test: Union[pyunit.TestCase, pyunit.TestSuite]) -> itrial.IReporter: """ Run the test or suite and return a result object. """ @@ -881,7 +903,9 @@ def run(self, test: ITestCase) -> itrial.IReporter: return run(test, self._forceGarbageCollection) def _runWithoutDecoration( - self, test: ITestCase, forceGarbageCollection: bool = False + self, + test: Union[pyunit.TestCase, pyunit.TestSuite], + forceGarbageCollection: bool = False, ) -> itrial.IReporter: """ Private helper that runs the given test but doesn't decorate it. @@ -909,7 +933,9 @@ def _runWithoutDecoration( result.done() return result - def runUntilFailure(self, test: ITestCase) -> itrial.IReporter: + def runUntilFailure( + self, test: Union[pyunit.TestCase, pyunit.TestSuite] + ) -> itrial.IReporter: """ Repeatedly run C{test} until it fails. """ @@ -918,6 +944,9 @@ def runUntilFailure(self, test: ITestCase) -> itrial.IReporter: count += 1 self.stream.write("Test Pass %d\n" % (count,)) if count == 1: + # If test is a TestSuite, run *mutates it*. So only follow + # this code-path once! Otherwise the decorations accumulate + # forever. result = self.run(test) else: result = self._runWithoutDecoration(test) From 3d70ae83964ebb30fb426b0b88cd7546610c095b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 21:13:21 -0400 Subject: [PATCH 04/10] Remove some unused attributes --- src/twisted/scripts/trial.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/twisted/scripts/trial.py b/src/twisted/scripts/trial.py index 38e0c971d51..531fe46ce17 100644 --- a/src/twisted/scripts/trial.py +++ b/src/twisted/scripts/trial.py @@ -20,7 +20,7 @@ from twisted.python import failure, reflect, usage from twisted.python.filepath import FilePath from twisted.python.reflect import namedModule -from twisted.trial import itrial, reporter, runner +from twisted.trial import itrial, runner from twisted.trial._dist.disttrial import DistTrialRunner from twisted.trial.unittest import TestSuite @@ -235,7 +235,6 @@ class _BasicOptions: ], ) - fallbackReporter = reporter.TreeReporter tracer: Optional[trace.Trace] = None def __init__(self): @@ -476,9 +475,6 @@ class Options(_BasicOptions, usage.Options, app.ReactorSelectionMixin): _workerFlags = ["disablegc", "force-gc", "coverage"] _workerParameters = ["recursionlimit", "reactor", "without-module"] - fallbackReporter = reporter.TreeReporter - extra = None - def opt_jobs(self, number): """ Number of local workers to run, a strictly positive integer. From f22b885befd8d5b2e5d6c53ba88fb70ffa437a70 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 21:17:54 -0400 Subject: [PATCH 05/10] news fragment --- src/twisted/newsfragments/11692.misc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/twisted/newsfragments/11692.misc diff --git a/src/twisted/newsfragments/11692.misc b/src/twisted/newsfragments/11692.misc new file mode 100644 index 00000000000..e69de29bb2d From 7da5aa0fd2bd610eec58505759572b35d497bd70 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 21:25:41 -0400 Subject: [PATCH 06/10] python 3.7 compat --- src/twisted/trial/runner.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index aa94f9e1db6..3483ebf5920 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -37,22 +37,12 @@ import warnings from contextlib import contextmanager from importlib.machinery import SourceFileLoader -from typing import ( - Any, - Callable, - Generator, - List, - Optional, - Protocol, - TextIO, - Type, - Union, -) +from typing import Any, Callable, Generator, List, Optional, TextIO, Type, Union from zope.interface import implementer from attrs import define -from typing_extensions import ParamSpec, TypeAlias, TypeGuard +from typing_extensions import ParamSpec, Protocol, TypeAlias, TypeGuard from twisted.internet import defer from twisted.python import failure, filepath, log, modules, reflect From fedc480b498ee6d40b9585a8ae244bc3101b4d95 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 17:08:24 -0400 Subject: [PATCH 07/10] _result is no longer used --- src/twisted/trial/runner.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index 3483ebf5920..6976a8c0ad6 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -37,7 +37,7 @@ import warnings from contextlib import contextmanager from importlib.machinery import SourceFileLoader -from typing import Any, Callable, Generator, List, Optional, TextIO, Type, Union +from typing import Callable, Generator, List, Optional, TextIO, Type, Union from zope.interface import implementer @@ -859,8 +859,6 @@ class TrialRunner: debugger: Optional[_Debugger] = None _exitFirst: bool = False - _result: Optional[Any] = None - _log: log.LogPublisher = log # type: ignore[assignment] def _makeResult(self) -> itrial.IReporter: From 730d53530c01c3148d85940663a0f919dbbe2b63 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 17:08:30 -0400 Subject: [PATCH 08/10] docs for runner instance vars --- src/twisted/trial/runner.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index 6976a8c0ad6..5bc75de41a7 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -841,6 +841,42 @@ def runUntilFailure( class TrialRunner: """ A specialised runner that the trial front end uses. + + @ivar reporterFactory: A callable to create a reporter to use. + + @ivar mode: Either C{None} for a normal test run, L{TrialRunner.DEBUG} for + a run in the debugger, or L{TrialRunner.DRY_RUN} to collect and report + the tests but not call any of them. + + @ivar logfile: The path to the file to write the test run log. + + @ivar stream: The file to report results to. + + @ivar profile: C{True} to run the tests with a profiler enabled. + + @ivar _tracebackFormat: A format name to use with L{Failure} for reporting + failures. + + @ivar _realTimeErrors: C{True} if errors should be reported as they + happen. C{False} if they should only be reported at the end of the + test run in the summary. + + @ivar uncleanWarnings: C{True} to report dirty reactor errors as warnings, + C{False} to report them as test-failing errors. + + @ivar workingDirectory: A path template to a directory which will be the + process's working directory while the tests are running. + + @ivar _forceGarbageCollection: C{True} to perform a full garbage + collection at least after each test. C{False} to let garbage + collection run only when it normally would. + + @ivar debugger: In debug mode, an object to use to launch the debugger. + + @ivar _exitFirst: C{True} to stop after the first failed test. C{False} + to run the whole suite. + + @ivar log: An object to give to the reporter to use as a log publisher. """ DEBUG = "debug" From 5ab99ae0ccaf30c106d65ee8f732dbfafd37751e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 17:09:42 -0400 Subject: [PATCH 09/10] expand the link --- src/twisted/trial/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/twisted/trial/util.py b/src/twisted/trial/util.py index 53ff516e7c8..f5450927617 100644 --- a/src/twisted/trial/util.py +++ b/src/twisted/trial/util.py @@ -240,7 +240,7 @@ def suppress(action="ignore", **kwarg): # This should be deleted, and replaced with twisted.application's code; see -# #6016: +# https://github.com/twisted/twisted/issues/6016: _P = ParamSpec("_P") _T = TypeVar("_T") From e5bb8f9a848cae3134f2319f5a131b5ca083c344 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 17:52:15 -0400 Subject: [PATCH 10/10] Favor human readers over mypy --- src/twisted/trial/runner.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/twisted/trial/runner.py b/src/twisted/trial/runner.py index 5bc75de41a7..ffc554e25c8 100644 --- a/src/twisted/trial/runner.py +++ b/src/twisted/trial/runner.py @@ -286,13 +286,19 @@ def name(thing: _Loadable) -> str: """ if isinstance(thing, pyunit.TestCase): return thing.id() - elif isinstance(thing, (modules.PythonAttribute, modules.PythonModule)): + + if isinstance(thing, (modules.PythonAttribute, modules.PythonModule)): return thing.name - elif isTestCase(thing): + + if isTestCase(thing): # TestCase subclass return reflect.qual(thing) - else: - assert False + + # Based on the type of thing, this is unreachable. Maybe someone calls + # this from un-type-checked code though. Also, even with the type + # information, mypy fails to determine this is unreachable and complains + # about a missing return without _something_ here. + raise TypeError(f"Cannot name {thing!r}") def isTestCase(obj: type) -> TypeGuard[Type[pyunit.TestCase]]: