From 81e94e87b3e41bca626b1beeb9397c0e1a438063 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 24 Aug 2022 16:18:50 -0400 Subject: [PATCH 01/31] add HasSum, IsSequenceOf, isFailure, similarFrame, and isTuple matchers These are handy for writing hamcrest-based tests for a lot of trial code. --- setup.cfg | 1 + src/twisted/trial/_dist/test/matchers.py | 184 +++++++++++++++++- src/twisted/trial/_dist/test/test_matchers.py | 107 ++++++++++ 3 files changed, 285 insertions(+), 7 deletions(-) create mode 100644 src/twisted/trial/_dist/test/test_matchers.py diff --git a/setup.cfg b/setup.cfg index 1c567871501..18c01766fab 100644 --- a/setup.cfg +++ b/setup.cfg @@ -51,6 +51,7 @@ packages = find: test = cython-test-exception-raiser >= 1.0.2, <2 PyHamcrest >= 1.9.0 + hypothesis ~= 6.0 ; List of dependencies required to build the documentation and test the ; release scripts and process. diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index 44a34eb4038..03bf77b4bae 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -5,18 +5,58 @@ Hamcrest matchers useful throughout the test suite. """ -from hamcrest import equal_to, has_length, has_properties +__all__ = [ + "matches_result", + "HasSum", + "IsSequenceOf", +] + +from typing import List, Protocol, Sequence, Sized, Tuple, TypeVar, cast + +from hamcrest import ( + contains_exactly, + contains_string, + equal_to, + has_length, + has_properties, + instance_of, +) +from hamcrest.core.base_matcher import BaseMatcher +from hamcrest.core.core.allof import AllOf +from hamcrest.core.description import Description from hamcrest.core.matcher import Matcher +from twisted.python.failure import Failure +from twisted.trial.reporter import TestResult + +T = TypeVar("T") + + +class Semigroup(Protocol[T]): + """ + A type with an associative binary operator. + + Common examples of a semigroup are integers with addition and strings with + concatenation. + """ + + def __add__(self, other: T) -> T: + """ + This must be associative: a + (b + c) == (a + b) + c + """ + + +S = TypeVar("S", bound=Semigroup) + def matches_result( successes: Matcher = equal_to(0), - errors: Matcher = has_length(0), - failures: Matcher = has_length(0), - skips: Matcher = has_length(0), - expectedFailures: Matcher = has_length(0), - unexpectedSuccesses: Matcher = has_length(0), -) -> Matcher: + errors: Matcher[Sized] = has_length(0), + failures: Matcher[Sized] = has_length(0), + skips: Matcher[Sized] = has_length(0), + expectedFailures: Matcher[Sized] = has_length(0), + unexpectedSuccesses: Matcher[Sized] = has_length(0), +) -> Matcher[TestResult]: """ Match a L{TestCase} instances with matching attributes. """ @@ -30,3 +70,133 @@ def matches_result( "unexpectedSuccesses": unexpectedSuccesses, } ) + + +class HasSum(BaseMatcher[Sequence[S]]): + """ + Match a sequence the elements of which sum to a value matched by + another matcher. + + :ivar sumMatcher: The matcher which must match the sum. + :ivar zero: The zero value for the matched type. + """ + + def __init__(self, sumMatcher: Matcher[S], zero: S) -> None: + self.sumMatcher = sumMatcher + self.zero = zero + + def _sum(self, sequence: Sequence[S]) -> S: + if not sequence: + return self.zero + elif all(isinstance(e, bytes) for e in sequence): + return cast(S, b"".join(cast(Sequence[bytes], sequence))) + elif all(isinstance(e, str) for e in sequence): + return cast(S, "".join(cast(Sequence[str], sequence))) + else: + return sum(sequence, self.zero) + + def _matches(self, item: Sequence[S]) -> bool: + """ + Determine whether the sum of the sequence is matched. + """ + s = self._sum(item) + return self.sumMatcher.matches(s) + + def describe_mismatch(self, item: Sequence[S], description: Description) -> None: + """ + Describe the mismatch. + """ + s = self._sum(item) + description.append_description_of(self) + self.sumMatcher.describe_mismatch(s, description) + return None + + def describe_to(self, description: Description) -> None: + """ + Describe this matcher for error messages. + """ + description.append_text("a sequence with sum ") + description.append_description_of(self.sumMatcher) + description.append_text(", ") + + +class IsSequenceOf(BaseMatcher[Sequence[T]]): + """ + Match a sequence where every element is matched by another matcher. + + :ivar elementMatcher: The matcher which must match every element of the + sequence. + """ + + def __init__(self, elementMatcher: Matcher[T]) -> None: + self.elementMatcher = elementMatcher + + def _matches(self, item: Sequence[T]) -> bool: + """ + Determine whether every element of the sequence is matched. + """ + for elem in item: + if not self.elementMatcher.matches(elem): + return False + return True + + def describe_mismatch(self, item: Sequence[T], description: Description) -> None: + """ + Describe the mismatch. + """ + for idx, elem in enumerate(item): + if not self.elementMatcher.matches(elem): + description.append_description_of(self) + description.append_text(f"not sequence with element #{idx} {elem!r}") + + def describe_to(self, description: Description) -> None: + """ + Describe this matcher for error messages. + """ + description.append_text("a sequence containing only ") + description.append_description_of(self.elementMatcher) + description.append_text(", ") + + +def isFailure(**properties: Matcher[object]) -> Matcher[object]: + """ + Match an instance of L{Failure} with matching attributes. + """ + return AllOf( + instance_of(Failure), + has_properties(**properties), + ) + + +def similarFrame( + functionName: str, fileName: str +) -> Matcher[Sequence[Tuple[str, str, int, List[object], List[object]]]]: + """ + Match a tuple representation of a frame like those used by + L{twisted.python.failure.Failure}. + """ + # The frames depend on exact layout of the source + # code in files and on the filesystem so we won't + # bother being very precise here. Just verify we + # see some distinctive fragments. + # + # In particular, the last frame should be a tuple like + # + # (functionName, fileName, someint, [], []) + return contains_exactly( + equal_to("test_error"), + contains_string("pyunitcases.py"), # type: ignore[arg-type] + instance_of(int), # type: ignore[arg-type] + equal_to([]), + equal_to([]), + ) + +def isTuple(*matchers: Matcher[object]) -> Matcher[object]: + """ + Match tuples for which the elements are matched by corresponding + elements of a tuple of matchers. + """ + return AllOf( + instance_of(tuple), + contains_exactly(*matchers), # type: ignore[arg-type] + ) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py new file mode 100644 index 00000000000..0550aa37b83 --- /dev/null +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -0,0 +1,107 @@ +""" +Tests for L{twisted.trial._dist.test.matchers}. +""" + +from typing import Sequence + +from hamcrest import anything, assert_that, contains_string, equal_to, is_, not_ +from hamcrest.core.core.allof import AllOf +from hamcrest.core.matcher import Matcher +from hamcrest.core.string_description import StringDescription +from hypothesis import given +from hypothesis.strategies import booleans, integers, just, lists + +from twisted.trial.unittest import SynchronousTestCase +from .matchers import HasSum, IsSequenceOf, S + + +class HasSumTests(SynchronousTestCase): + """ + Tests for L{HasSum}. + """ + + sequences = lists(integers()) + # For the moment we know we always have integers + zeros = just(0) + + @given(sequences, zeros) + def test_matches(self, seq: Sequence[S], zero: S) -> None: + """ + L{HasSum} matches a sequence if the elements sum to a value matched by + the parameterized matcher. + """ + expected = sum(seq, zero) + + description = StringDescription() + matcher = HasSum(equal_to(expected), zero) + assert_that(matcher.matches(seq, description), equal_to(True)) + assert_that(str(description), equal_to("")) + + @given(sequences, zeros) + def test_mismatches(self, seq: Sequence[S], zero: S) -> None: + """ + L{HasSum} does not match a sequence if the elements do not sum to a + value matched by the parameterized matcher. + """ + # A matcher that never matches. + sumMatcher: Matcher[S] = not_(anything()) + + actualDescription = StringDescription() + matcher = HasSum(sumMatcher, zero) + assert_that(matcher.matches(seq, actualDescription), equal_to(False)) + + sumMatcherDescription = StringDescription() + sumMatcherDescription.append_description_of(sumMatcher) + + assert_that( + str(actualDescription), + is_( + AllOf( + contains_string("a sequence with sum"), + contains_string(str(sumMatcherDescription)), + ) + ), + ) + + +class IsSequenceOfTests(SynchronousTestCase): + """ + Tests for L{IsSequenceOf}. + """ + + sequences = lists(booleans()) + + @given(integers(min_value=0, max_value=1000)) + def test_matches(self, num_items: int) -> None: + """ + L{IsSequenceOf} matches a sequence if all of the elements are + matched by the parameterized matcher. + """ + seq = [True] * num_items + matcher = IsSequenceOf(equal_to(True)) + + actualDescription = StringDescription() + assert_that(matcher.matches(seq, actualDescription), equal_to(True)) + assert_that(str(actualDescription), equal_to("")) + + @given(integers(min_value=0, max_value=1000), integers(min_value=0, max_value=1000)) + def test_mismatches(self, num_before: int, num_after: int) -> None: + """ + L{IsSequenceOf} does not match a sequence if any of the elements + are not matched by the parameterized matcher. + """ + # Hide the non-matching value somewhere in the sequence. + seq = [True] * num_before + [False] + [True] * num_after + matcher = IsSequenceOf(equal_to(True)) + + actualDescription = StringDescription() + assert_that(matcher.matches(seq, actualDescription), equal_to(False)) + assert_that( + str(actualDescription), + is_( + AllOf( + contains_string("a sequence containing only"), + contains_string(f"not sequence with element #{num_before}"), + ) + ), + ) From f6c57c2bc37418c5b17a663dc5ca96f62303fe0b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 16:48:38 -0400 Subject: [PATCH 02/31] Tests for isTuple --- src/twisted/trial/_dist/test/test_matchers.py | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index 0550aa37b83..d59f56dd811 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -9,10 +9,10 @@ from hamcrest.core.matcher import Matcher from hamcrest.core.string_description import StringDescription from hypothesis import given -from hypothesis.strategies import booleans, integers, just, lists +from hypothesis.strategies import booleans, integers, just, lists, one_of, text, binary from twisted.trial.unittest import SynchronousTestCase -from .matchers import HasSum, IsSequenceOf, S +from .matchers import HasSum, IsSequenceOf, S, isTuple class HasSumTests(SynchronousTestCase): @@ -105,3 +105,53 @@ def test_mismatches(self, num_before: int, num_after: int) -> None: ) ), ) + + +class IsTupleTests(SynchronousTestCase): + """ + Tests for L{isTuple}. + """ + @given(lists(integers(), min_size=0, max_size=10)) + def test_matches(self, elements: list[int]) -> None: + """ + L{isTuple} matches tuples if they have the same number of elements + as the number of matchers given and each element is matched by the + corresponding matcher. + """ + matcher = isTuple(*(equal_to(e) for e in elements)) + actualDescription = StringDescription() + assert_that(matcher.matches(tuple(elements), actualDescription), equal_to(True)) + assert_that(str(actualDescription), equal_to("")) + + @given( + lists(integers(), min_size=0, max_size=10), + integers(), + lists(integers(), min_size=0, max_size=10), + ) + def test_mismatch(self, before: list[int], mismatch: int, after: list[int]) -> None: + """ + L{isTuple} does not match if any element is not matched. + """ + matchers = [equal_to(e) for e in before] + matchers.append(not_(anything())) + matchers = [equal_to(e) for e in after] + matcher = isTuple(*matchers) + + elements = tuple(before) + (mismatch,) + tuple(after) + actualDescription = StringDescription() + assert_that(matcher.matches(elements, actualDescription), equal_to(False)) + + @given( + one_of( + lists(integers(), max_size=2), + text(max_size=2), + binary(max_size=2), + integers(), + ), + ) + def test_mismatchOtherType(self, wrongType: object) -> None: + """ + L{isTuple} does not match non-tuple values. + """ + matcher = isTuple(anything()) + assert_that(matcher.matches([1]), equal_to(False)) From 0c9f6a29341e9560f9ef88836eb9e9f77a1a8363 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 17:02:02 -0400 Subject: [PATCH 03/31] Tests for isFailure and some bug fixes --- src/twisted/trial/_dist/test/matchers.py | 13 ++-- src/twisted/trial/_dist/test/test_matchers.py | 77 +++++++++++++++++-- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index 03bf77b4bae..e374fe884c6 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -184,13 +184,16 @@ def similarFrame( # # (functionName, fileName, someint, [], []) return contains_exactly( - equal_to("test_error"), - contains_string("pyunitcases.py"), # type: ignore[arg-type] + equal_to(functionName), + contains_string(fileName), # type: ignore[arg-type] instance_of(int), # type: ignore[arg-type] - equal_to([]), - equal_to([]), + # Unfortunately Failure makes them sometimes tuples, sometimes + # dict_items. + has_length(0), # type: ignore[arg-type] + has_length(0), # type: ignore[arg-type] ) + def isTuple(*matchers: Matcher[object]) -> Matcher[object]: """ Match tuples for which the elements are matched by corresponding @@ -198,5 +201,5 @@ def isTuple(*matchers: Matcher[object]) -> Matcher[object]: """ return AllOf( instance_of(tuple), - contains_exactly(*matchers), # type: ignore[arg-type] + contains_exactly(*matchers), # type: ignore[arg-type] ) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index d59f56dd811..527aea35ed6 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -2,17 +2,35 @@ Tests for L{twisted.trial._dist.test.matchers}. """ -from typing import Sequence - -from hamcrest import anything, assert_that, contains_string, equal_to, is_, not_ +from typing import Sequence, Type + +from hamcrest import ( + anything, + assert_that, + contains, + contains_string, + equal_to, + is_, + not_, +) from hamcrest.core.core.allof import AllOf from hamcrest.core.matcher import Matcher from hamcrest.core.string_description import StringDescription from hypothesis import given -from hypothesis.strategies import booleans, integers, just, lists, one_of, text, binary - +from hypothesis.strategies import ( + binary, + booleans, + integers, + just, + lists, + one_of, + sampled_from, + text, +) + +from twisted.python.failure import Failure from twisted.trial.unittest import SynchronousTestCase -from .matchers import HasSum, IsSequenceOf, S, isTuple +from .matchers import HasSum, IsSequenceOf, S, isFailure, isTuple, similarFrame class HasSumTests(SynchronousTestCase): @@ -111,6 +129,7 @@ class IsTupleTests(SynchronousTestCase): """ Tests for L{isTuple}. """ + @given(lists(integers(), min_size=0, max_size=10)) def test_matches(self, elements: list[int]) -> None: """ @@ -155,3 +174,49 @@ def test_mismatchOtherType(self, wrongType: object) -> None: """ matcher = isTuple(anything()) assert_that(matcher.matches([1]), equal_to(False)) + + +class IsFailureTests(SynchronousTestCase): + """ + Tests for L{isFailure}. + """ + + @given(sampled_from([ValueError, ZeroDivisionError, RuntimeError])) + def test_matches(self, excType: Type[BaseException]) -> None: + """ + L{isFailure} matches instances of L{Failure} with matching + attributes. + """ + matcher = isFailure(type=equal_to(excType)) + failure = Failure(excType()) + assert_that(matcher.matches(failure), equal_to(True)) + + @given(sampled_from([ValueError, ZeroDivisionError, RuntimeError])) + def test_mismatches(self, excType: Type[BaseException]) -> None: + """ + L{isFailure} does not match match instances of L{Failure} with + attributes that don't match. + """ + matcher = isFailure(type=equal_to(excType), other=not_(anything())) + failure = Failure(excType()) + assert_that(matcher.matches(failure), equal_to(False)) + + def test_frames(self): + """ + The L{similarFrame} matcher matches elements of the C{frames} list + of a L{Failure}. + """ + try: + raise ValueError("Oh no") + except BaseException: + f = Failure() + + actualDescription = StringDescription() + matcher = isFailure( + frames=contains(similarFrame("test_frames", "test_matchers")) + ) + assert_that( + matcher.matches(f, actualDescription), + equal_to(True), + str(actualDescription), + ) From f004be9207850e84e1e725df7e5a40e2b5c68bba Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 16:13:52 -0400 Subject: [PATCH 04/31] Fix WorkerProtocolTests and LocalWorkerAMPTests The previous implementation used unnecessary fakes and encoded unnecessary implementation details as well as unnecessarily duplicating in-memory ITransport implementation available in other testing libraries in Twisted. This is an almost-no-behavior-change commit that just makes the tests simpler and exercises the code under test in a more realistic way. --- src/twisted/trial/_dist/test/test_worker.py | 308 ++++++++------------ src/twisted/trial/_dist/worker.py | 2 +- src/twisted/trial/_synctest.py | 25 +- 3 files changed, 142 insertions(+), 193 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index 03c6a857a54..66a9ff546bc 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -7,19 +7,20 @@ import os from io import BytesIO, StringIO +from typing import Type +from unittest import TestCase as PyUnitTestCase from zope.interface.verify import verifyObject -from twisted.internet.defer import fail, succeed +from hamcrest import assert_that, contains_exactly, equal_to, has_item, has_properties + +from twisted.internet.defer import fail from twisted.internet.error import ProcessDone from twisted.internet.interfaces import IAddress, ITransport -from twisted.protocols.amp import AMP from twisted.python.failure import Failure from twisted.python.filepath import FilePath -from twisted.python.reflect import fullyQualifiedName -from twisted.scripts import trial -from twisted.test.proto_helpers import StringTransport -from twisted.trial._dist import managercommands, workercommands +from twisted.test.iosim import connectedServerAndClient +from twisted.trial._dist import managercommands from twisted.trial._dist.worker import ( LocalWorker, LocalWorkerAMP, @@ -29,13 +30,9 @@ WorkerProtocol, ) from twisted.trial.reporter import TestResult -from twisted.trial.unittest import TestCase - - -class FakeAMP(AMP): - """ - A fake amp protocol. - """ +from twisted.trial.test import pyunitcases, skipping +from twisted.trial.unittest import TestCase, makeTodo +from .matchers import isFailure, isTuple, matches_result, similarFrame class WorkerProtocolTests(TestCase): @@ -43,41 +40,34 @@ class WorkerProtocolTests(TestCase): Tests for L{WorkerProtocol}. """ - def setUp(self): + worker: WorkerProtocol + server: LocalWorkerAMP + + def setUp(self) -> None: """ Set up a transport, a result stream and a protocol instance. """ - self.serverTransport = StringTransport() - self.clientTransport = StringTransport() - self.server = WorkerProtocol() - self.server.makeConnection(self.serverTransport) - self.client = FakeAMP() - self.client.makeConnection(self.clientTransport) + self.worker, self.server, pump = connectedServerAndClient( + LocalWorkerAMP, WorkerProtocol, greet=False + ) + self.flush = pump.flush - def test_run(self): + def test_run(self) -> None: """ - Calling the L{workercommands.Run} command on the client returns a + Sending the L{workercommands.Run} command to the worker returns a response with C{success} sets to C{True}. """ - d = self.client.callRemote(workercommands.Run, testCase="doesntexist") - - def check(result): - self.assertTrue(result["success"]) + d = self.server.run(pyunitcases.PyUnitTest("test_pass"), TestResult()) + self.flush() + self.assertEqual({"success": True}, self.successResultOf(d)) - d.addCallback(check) - self.server.dataReceived(self.clientTransport.value()) - self.clientTransport.clear() - self.client.dataReceived(self.serverTransport.value()) - self.serverTransport.clear() - return d - - def test_start(self): + def test_start(self) -> None: """ The C{start} command changes the current path. """ curdir = os.path.realpath(os.path.curdir) self.addCleanup(os.chdir, curdir) - self.server.start("..") + self.worker.start("..") self.assertNotEqual(os.path.realpath(os.path.curdir), curdir) @@ -86,197 +76,155 @@ class LocalWorkerAMPTests(TestCase): Test case for distributed trial's manager-side local worker AMP protocol """ - def setUp(self): - self.managerTransport = StringTransport() - self.managerAMP = LocalWorkerAMP() - self.managerAMP.makeConnection(self.managerTransport) - self.result = TestResult() - self.workerTransport = StringTransport() - self.worker = AMP() - self.worker.makeConnection(self.workerTransport) - - config = trial.Options() - self.testName = "twisted.doesnexist" - config["tests"].append(self.testName) - self.testCase = trial._getSuite(config)._tests.pop() + def setUp(self) -> None: + self.worker, self.managerAMP, pump = connectedServerAndClient( + LocalWorkerAMP, WorkerProtocol, greet=False + ) + self.flush = pump.flush - self.managerAMP.run(self.testCase, self.result) - self.managerTransport.clear() + def workerRunTest( + self, testCase: PyUnitTestCase, makeResult: Type[TestResult] = TestResult + ) -> TestResult: + result = makeResult() + d = self.managerAMP.run(testCase, result) + self.flush() + self.assertEqual({"success": True}, self.successResultOf(d)) + return result - def pumpTransports(self): - """ - Sends data from C{self.workerTransport} to C{self.managerAMP}, and then - data from C{self.managerTransport} back to C{self.worker}. - """ - self.managerAMP.dataReceived(self.workerTransport.value()) - self.workerTransport.clear() - self.worker.dataReceived(self.managerTransport.value()) - - def test_runSuccess(self): + def test_runSuccess(self) -> None: """ Run a test, and succeed. """ - results = [] - - d = self.worker.callRemote(managercommands.AddSuccess, testName=self.testName) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() + result = self.workerRunTest(pyunitcases.PyUnitTest("test_pass")) + assert_that(result, matches_result(successes=equal_to(1))) - self.assertTrue(results) - - def test_runExpectedFailure(self): + def test_runExpectedFailure(self) -> None: """ Run a test, and fail expectedly. """ - results = [] - - d = self.worker.callRemote( - managercommands.AddExpectedFailure, - testName=self.testName, - error="error", - todo="todoReason", + case = skipping.SynchronousStrictTodo("test_todo1") + result = self.workerRunTest(case) + assert_that( + result, + matches_result( + expectedFailures=equal_to( + [ + # Match the strings used in the test we ran. + (case, "expected failure", makeTodo("todo1")), + ] + ) + ), ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - self.assertEqual(self.testCase, self.result.expectedFailures[0][0]) - self.assertTrue(results) - - def test_runError(self): + def test_runError(self) -> None: """ Run a test, and encounter an error. """ - results = [] - errorClass = fullyQualifiedName(ValueError) - d = self.worker.callRemote( - managercommands.AddError, - testName=self.testName, - error="error", - errorClass=errorClass, - frames=[], - ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - - case, failure = self.result.errors[0] - self.assertEqual(self.testCase, case) - self.assertEqual(failure.type, ValueError) - self.assertEqual(failure.value, WorkerException("error")) - self.assertTrue(results) - - def test_runErrorWithFrames(self): - """ - L{LocalWorkerAMP._buildFailure} recreates the C{Failure.frames} from - the C{frames} argument passed to C{AddError}. - """ - results = [] - errorClass = fullyQualifiedName(ValueError) - d = self.worker.callRemote( - managercommands.AddError, - testName=self.testName, - error="error", - errorClass=errorClass, - frames=["file.py", "invalid code", "3"], + case = pyunitcases.PyUnitTest("test_error") + result = self.workerRunTest(case) + assert_that( + result, + matches_result( + # Failures don't compare nicely so unpack the result and peek inside. + errors=contains_exactly( + isTuple( # type: ignore[arg-type] + equal_to(case), + isFailure( + type=equal_to(Exception), + value=equal_to(WorkerException("pyunit error")), + frames=has_item( # type: ignore[arg-type] + similarFrame("test_error", "pyunitcases.py") # type: ignore[arg-type] + ), + ), + ), + ), + ), ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - case, failure = self.result.errors[0] - self.assertEqual(self.testCase, case) - self.assertEqual(failure.type, ValueError) - self.assertEqual(failure.value, WorkerException("error")) - self.assertEqual([("file.py", "invalid code", 3, [], [])], failure.frames) - self.assertTrue(results) - - def test_runFailure(self): + def test_runFailure(self) -> None: """ Run a test, and fail. """ - results = [] - failClass = fullyQualifiedName(RuntimeError) - d = self.worker.callRemote( - managercommands.AddFailure, - testName=self.testName, - fail="fail", - failClass=failClass, - frames=[], + case = pyunitcases.PyUnitTest("test_fail") + result = self.workerRunTest(case) + assert_that( + result, + matches_result( + failures=contains_exactly( + isTuple( # type: ignore[arg-type] + equal_to(case), + isFailure( + # AssertionError is the type raised by TestCase.fail + type=equal_to(AssertionError), + value=equal_to(WorkerException("pyunit failure")), + ), + ), + ), + ), ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - - case, failure = self.result.failures[0] - self.assertEqual(self.testCase, case) - self.assertEqual(failure.type, RuntimeError) - self.assertEqual(failure.value, WorkerException("fail")) - self.assertTrue(results) - def test_runSkip(self): + def test_runSkip(self) -> None: """ Run a test, but skip it. """ - results = [] - - d = self.worker.callRemote( - managercommands.AddSkip, testName=self.testName, reason="reason" + case = pyunitcases.PyUnitTest("test_skip") + result = self.workerRunTest(case) + assert_that( + result, + matches_result( + skips=contains_exactly( # type: ignore[arg-type] + isTuple( # type: ignore[arg-type] + equal_to(case), + equal_to("pyunit skip"), + ), + ), + ), ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - - self.assertEqual(self.testCase, self.result.skips[0][0]) - self.assertTrue(results) - def test_runUnexpectedSuccesses(self): + def test_runUnexpectedSuccesses(self) -> None: """ Run a test, and succeed unexpectedly. """ - results = [] - - d = self.worker.callRemote( - managercommands.AddUnexpectedSuccess, testName=self.testName, todo="todo" + case = skipping.SynchronousStrictTodo("test_todo7") + result = self.workerRunTest(case) + assert_that( + result, + matches_result( + unexpectedSuccesses=contains_exactly( # type: ignore[arg-type] + isTuple( # type: ignore[arg-type] + equal_to(case), + equal_to("todo7"), + ), + ), + ), ) - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - - self.assertEqual(self.testCase, self.result.unexpectedSuccesses[0][0]) - self.assertTrue(results) - def test_testWrite(self): + def test_testWrite(self) -> None: """ L{LocalWorkerAMP.testWrite} writes the data received to its test stream. """ - results = [] stream = StringIO() self.managerAMP.setTestStream(stream) - - command = managercommands.TestWrite - d = self.worker.callRemote(command, out="Some output") - d.addCallback(lambda result: results.append(result["success"])) - self.pumpTransports() - + d = self.worker.callRemote(managercommands.TestWrite, out="Some output") + self.flush() + self.assertEqual({"success": True}, self.successResultOf(d)) self.assertEqual("Some output\n", stream.getvalue()) - self.assertTrue(results) - def test_stopAfterRun(self): + def test_stopAfterRun(self) -> None: """ L{LocalWorkerAMP.run} calls C{stopTest} on its test result once the C{Run} commands has succeeded. """ - result = object() - stopped = [] - - def fakeCallRemote(command, testCase): - return succeed(result) - - self.managerAMP.callRemote = fakeCallRemote class StopTestResult(TestResult): - def stopTest(self, test): - stopped.append(test) + _stopped = False + + def stopTest(self, test: PyUnitTestCase) -> None: + self._stopped = True - d = self.managerAMP.run(self.testCase, StopTestResult()) - self.assertEqual([self.testCase], stopped) - return d.addCallback(self.assertIdentical, result) + result = self.workerRunTest(pyunitcases.PyUnitTest("test_pass"), StopTestResult) + assert_that(result, has_properties(_stopped=equal_to(True))) class SpyDataLocalWorkerAMP(LocalWorkerAMP): diff --git a/src/twisted/trial/_dist/worker.py b/src/twisted/trial/_dist/worker.py index 9febc000d5f..37a9a409791 100644 --- a/src/twisted/trial/_dist/worker.py +++ b/src/twisted/trial/_dist/worker.py @@ -188,7 +188,7 @@ def addExpectedFailure( """ Add an expected failure to the reporter. """ - _todo = Todo(todo) + _todo = Todo("" if todo is None else todo) self._result.addExpectedFailure(self._testCase, error, _todo) return {"success": True} diff --git a/src/twisted/trial/_synctest.py b/src/twisted/trial/_synctest.py index 8969e642971..77f14422d20 100644 --- a/src/twisted/trial/_synctest.py +++ b/src/twisted/trial/_synctest.py @@ -17,11 +17,13 @@ import unittest as pyunit import warnings from dis import findlinestarts as _findlinestarts -from typing import List, NoReturn, Optional, Tuple, TypeVar, Union +from typing import Iterable, List, NoReturn, Optional, Tuple, Type, TypeVar, Union # Python 2.7 and higher has skip support built-in from unittest import SkipTest +from attrs import frozen + from twisted.internet.defer import Deferred, ensureDeferred from twisted.python import failure, log, monkey from twisted.python.deprecate import ( @@ -43,6 +45,7 @@ class FailTest(AssertionError): """ +@frozen class Todo: """ Internal object used to mark a L{TestCase} as 'todo'. Tests marked 'todo' @@ -50,19 +53,17 @@ class Todo: they do not fail the suite and the errors are reported in a separate category. If todo'd tests succeed, Trial L{TestResult}s will report an unexpected success. - """ - def __init__(self, reason, errors=None): - """ - @param reason: A string explaining why the test is marked 'todo' + @ivar reason: A string explaining why the test is marked 'todo' - @param errors: An iterable of exception types that the test is - expected to raise. If one of these errors is raised by the test, it - will be trapped. Raising any other kind of error will fail the test. - If L{None} is passed, then all errors will be trapped. - """ - self.reason = reason - self.errors = errors + @ivar errors: An iterable of exception types that the test is expected to + raise. If one of these errors is raised by the test, it will be + trapped. Raising any other kind of error will fail the test. If + L{None} then all errors will be trapped. + """ + + reason: str + errors: Optional[Iterable[Type[BaseException]]] = None def __repr__(self) -> str: return f"" From ed56326f926e51cba15a1eb1b5d426e9458ef2c4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 17:04:46 -0400 Subject: [PATCH 05/31] news fragment --- src/twisted/newsfragments/11616.misc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/twisted/newsfragments/11616.misc diff --git a/src/twisted/newsfragments/11616.misc b/src/twisted/newsfragments/11616.misc new file mode 100644 index 00000000000..e69de29bb2d From 1f60522e1677ae98452a9e8040b90fa7c9cde37b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 21:35:41 -0400 Subject: [PATCH 06/31] use typing.List to support more Python versions --- src/twisted/trial/_dist/test/test_matchers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index 527aea35ed6..a8fa9e65296 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -2,7 +2,7 @@ Tests for L{twisted.trial._dist.test.matchers}. """ -from typing import Sequence, Type +from typing import List, Sequence, Type from hamcrest import ( anything, @@ -131,7 +131,7 @@ class IsTupleTests(SynchronousTestCase): """ @given(lists(integers(), min_size=0, max_size=10)) - def test_matches(self, elements: list[int]) -> None: + def test_matches(self, elements: List[int]) -> None: """ L{isTuple} matches tuples if they have the same number of elements as the number of matchers given and each element is matched by the @@ -147,7 +147,7 @@ def test_matches(self, elements: list[int]) -> None: integers(), lists(integers(), min_size=0, max_size=10), ) - def test_mismatch(self, before: list[int], mismatch: int, after: list[int]) -> None: + def test_mismatch(self, before: List[int], mismatch: int, after: List[int]) -> None: """ L{isTuple} does not match if any element is not matched. """ From d949c89705f0ad452cd0482dc027dfa114569449 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 25 Aug 2022 21:38:00 -0400 Subject: [PATCH 07/31] switch to typing_extensions.Protocol to support more Python versions --- src/twisted/trial/_dist/test/matchers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index e374fe884c6..a6a91434042 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -11,7 +11,7 @@ "IsSequenceOf", ] -from typing import List, Protocol, Sequence, Sized, Tuple, TypeVar, cast +from typing import List, Sequence, Sized, Tuple, TypeVar, cast from hamcrest import ( contains_exactly, @@ -25,6 +25,7 @@ from hamcrest.core.core.allof import AllOf from hamcrest.core.description import Description from hamcrest.core.matcher import Matcher +from typing_extensions import Protocol from twisted.python.failure import Failure from twisted.trial.reporter import TestResult From e1eac51fd5429d74711c663947fe8e49d8ce399b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Aug 2022 11:39:25 -0400 Subject: [PATCH 08/31] some changes which hopefully improve readability --- src/twisted/trial/_dist/test/matchers.py | 12 +- src/twisted/trial/_dist/test/test_matchers.py | 124 +++++++++++------- 2 files changed, 82 insertions(+), 54 deletions(-) diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index a6a91434042..1631d02f679 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -11,7 +11,7 @@ "IsSequenceOf", ] -from typing import List, Sequence, Sized, Tuple, TypeVar, cast +from typing import List, Sequence, Sized, Tuple, TypeVar from hamcrest import ( contains_exactly, @@ -89,12 +89,10 @@ def __init__(self, sumMatcher: Matcher[S], zero: S) -> None: def _sum(self, sequence: Sequence[S]) -> S: if not sequence: return self.zero - elif all(isinstance(e, bytes) for e in sequence): - return cast(S, b"".join(cast(Sequence[bytes], sequence))) - elif all(isinstance(e, str) for e in sequence): - return cast(S, "".join(cast(Sequence[str], sequence))) - else: - return sum(sequence, self.zero) + result = self.zero + for elem in sequence: + result = result + elem + return result def _matches(self, item: Sequence[S]) -> bool: """ diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index a8fa9e65296..ea4d1986728 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -2,18 +2,9 @@ Tests for L{twisted.trial._dist.test.matchers}. """ -from typing import List, Sequence, Type - -from hamcrest import ( - anything, - assert_that, - contains, - contains_string, - equal_to, - is_, - not_, -) -from hamcrest.core.core.allof import AllOf +from typing import Callable, List, Sequence, Tuple, Type + +from hamcrest import anything, assert_that, contains, contains_string, equal_to, not_ from hamcrest.core.matcher import Matcher from hamcrest.core.string_description import StringDescription from hypothesis import given @@ -26,60 +17,76 @@ one_of, sampled_from, text, + tuples, ) from twisted.python.failure import Failure from twisted.trial.unittest import SynchronousTestCase from .matchers import HasSum, IsSequenceOf, S, isFailure, isTuple, similarFrame +Summer = Callable[[Sequence[S]], S] +concatInt = sum +concatStr = "".join +concatBytes = b"".join + class HasSumTests(SynchronousTestCase): """ Tests for L{HasSum}. """ - sequences = lists(integers()) - # For the moment we know we always have integers - zeros = just(0) + summables = one_of( + tuples(lists(integers()), just(concatInt)), + tuples(lists(text()), just(concatStr)), + tuples(lists(binary()), just(concatBytes)), + ) - @given(sequences, zeros) - def test_matches(self, seq: Sequence[S], zero: S) -> None: + @given(summables) + def test_matches(self, summable: Tuple[Sequence[S], Summer[S]]) -> None: """ L{HasSum} matches a sequence if the elements sum to a value matched by the parameterized matcher. + + :param summable: A tuple of a sequence of values to try to match and a + function which can compute the correct sum for that sequence. """ - expected = sum(seq, zero) + seq, sumFunc = summable + expected = sumFunc(seq) + zero = sumFunc([]) + matcher = HasSum(equal_to(expected), zero) description = StringDescription() - matcher = HasSum(equal_to(expected), zero) assert_that(matcher.matches(seq, description), equal_to(True)) assert_that(str(description), equal_to("")) - @given(sequences, zeros) - def test_mismatches(self, seq: Sequence[S], zero: S) -> None: + @given(summables) + def test_mismatches( + self, + summable: Tuple[ + Sequence[S], + Summer[S], + ], + ) -> None: """ L{HasSum} does not match a sequence if the elements do not sum to a value matched by the parameterized matcher. + + :param summable: See L{test_matches}. """ + seq, sumFunc = summable + zero = sumFunc([]) # A matcher that never matches. sumMatcher: Matcher[S] = not_(anything()) + matcher = HasSum(sumMatcher, zero) actualDescription = StringDescription() - matcher = HasSum(sumMatcher, zero) assert_that(matcher.matches(seq, actualDescription), equal_to(False)) sumMatcherDescription = StringDescription() sumMatcherDescription.append_description_of(sumMatcher) - - assert_that( - str(actualDescription), - is_( - AllOf( - contains_string("a sequence with sum"), - contains_string(str(sumMatcherDescription)), - ) - ), - ) + actualStr = str(actualDescription) + assert_that(actualStr, contains_string("a sequence with sum")) + assert_that(actualStr, contains_string(str(sumMatcherDescription))) class IsSequenceOfTests(SynchronousTestCase): @@ -90,12 +97,14 @@ class IsSequenceOfTests(SynchronousTestCase): sequences = lists(booleans()) @given(integers(min_value=0, max_value=1000)) - def test_matches(self, num_items: int) -> None: + def test_matches(self, numItems: int) -> None: """ L{IsSequenceOf} matches a sequence if all of the elements are matched by the parameterized matcher. + + :param numItems: The length of a sequence to try to match. """ - seq = [True] * num_items + seq = [True] * numItems matcher = IsSequenceOf(equal_to(True)) actualDescription = StringDescription() @@ -103,25 +112,27 @@ def test_matches(self, num_items: int) -> None: assert_that(str(actualDescription), equal_to("")) @given(integers(min_value=0, max_value=1000), integers(min_value=0, max_value=1000)) - def test_mismatches(self, num_before: int, num_after: int) -> None: + def test_mismatches(self, numBefore: int, numAfter: int) -> None: """ L{IsSequenceOf} does not match a sequence if any of the elements are not matched by the parameterized matcher. + + :param numBefore: In the sequence to try to match, the number of + elements expected to match before an expected mismatch. + + :param numAfter: In the sequence to try to match, the number of + elements expected expected to match after an expected mismatch. """ # Hide the non-matching value somewhere in the sequence. - seq = [True] * num_before + [False] + [True] * num_after + seq = [True] * numBefore + [False] + [True] * numAfter matcher = IsSequenceOf(equal_to(True)) actualDescription = StringDescription() assert_that(matcher.matches(seq, actualDescription), equal_to(False)) + actualStr = str(actualDescription) + assert_that(actualStr, contains_string("a sequence containing only")) assert_that( - str(actualDescription), - is_( - AllOf( - contains_string("a sequence containing only"), - contains_string(f"not sequence with element #{num_before}"), - ) - ), + actualStr, contains_string(f"not sequence with element #{numBefore}") ) @@ -136,6 +147,9 @@ def test_matches(self, elements: List[int]) -> None: L{isTuple} matches tuples if they have the same number of elements as the number of matchers given and each element is matched by the corresponding matcher. + + :param elements: The elements with which to populate the tuple to + attempt to match with L{isTuple}. """ matcher = isTuple(*(equal_to(e) for e in elements)) actualDescription = StringDescription() @@ -150,6 +164,14 @@ def test_matches(self, elements: List[int]) -> None: def test_mismatch(self, before: List[int], mismatch: int, after: List[int]) -> None: """ L{isTuple} does not match if any element is not matched. + + :param before: For the tuple to match, elements leading up to an + expected mismatching element. + + :param mismatch: An element expected to mismatch. + + :param after: For the tuple to match, elements following an expected + mismatching element. """ matchers = [equal_to(e) for e in before] matchers.append(not_(anything())) @@ -168,12 +190,14 @@ def test_mismatch(self, before: List[int], mismatch: int, after: List[int]) -> N integers(), ), ) - def test_mismatchOtherType(self, wrongType: object) -> None: + def test_mismatchOtherType(self, mismatch: object) -> None: """ L{isTuple} does not match non-tuple values. + + :param mismatch: A value of a type other than tuple. """ matcher = isTuple(anything()) - assert_that(matcher.matches([1]), equal_to(False)) + assert_that(matcher.matches(mismatch), equal_to(False)) class IsFailureTests(SynchronousTestCase): @@ -186,6 +210,9 @@ def test_matches(self, excType: Type[BaseException]) -> None: """ L{isFailure} matches instances of L{Failure} with matching attributes. + + :param excType: An exception type to wrap in a L{Failure} to be + matched against. """ matcher = isFailure(type=equal_to(excType)) failure = Failure(excType()) @@ -194,8 +221,11 @@ def test_matches(self, excType: Type[BaseException]) -> None: @given(sampled_from([ValueError, ZeroDivisionError, RuntimeError])) def test_mismatches(self, excType: Type[BaseException]) -> None: """ - L{isFailure} does not match match instances of L{Failure} with + L{isFailure} does not match instances of L{Failure} with attributes that don't match. + + :param excType: An exception type to wrap in a L{Failure} to be + matched against. """ matcher = isFailure(type=equal_to(excType), other=not_(anything())) failure = Failure(excType()) @@ -218,5 +248,5 @@ def test_frames(self): assert_that( matcher.matches(f, actualDescription), equal_to(True), - str(actualDescription), + actualDescription, ) From e51fb63db245343c365fe777e0f056a9b27d3c07 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Aug 2022 11:50:04 -0400 Subject: [PATCH 09/31] Side-step pyhamcrest's nonsensical types --- src/twisted/trial/_dist/test/matchers.py | 15 +++++++-------- src/twisted/trial/_dist/test/test_worker.py | 12 ++++++------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index 1631d02f679..19ffb8a0055 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -11,7 +11,7 @@ "IsSequenceOf", ] -from typing import List, Sequence, Sized, Tuple, TypeVar +from typing import List, Sequence, Tuple, TypeVar from hamcrest import ( contains_exactly, @@ -28,7 +28,6 @@ from typing_extensions import Protocol from twisted.python.failure import Failure -from twisted.trial.reporter import TestResult T = TypeVar("T") @@ -52,12 +51,12 @@ def __add__(self, other: T) -> T: def matches_result( successes: Matcher = equal_to(0), - errors: Matcher[Sized] = has_length(0), - failures: Matcher[Sized] = has_length(0), - skips: Matcher[Sized] = has_length(0), - expectedFailures: Matcher[Sized] = has_length(0), - unexpectedSuccesses: Matcher[Sized] = has_length(0), -) -> Matcher[TestResult]: + errors: Matcher = has_length(0), + failures: Matcher = has_length(0), + skips: Matcher = has_length(0), + expectedFailures: Matcher = has_length(0), + unexpectedSuccesses: Matcher = has_length(0), +) -> Matcher: """ Match a L{TestCase} instances with matching attributes. """ diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index 66a9ff546bc..8e638c08f2e 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -127,7 +127,7 @@ def test_runError(self) -> None: matches_result( # Failures don't compare nicely so unpack the result and peek inside. errors=contains_exactly( - isTuple( # type: ignore[arg-type] + isTuple( equal_to(case), isFailure( type=equal_to(Exception), @@ -151,7 +151,7 @@ def test_runFailure(self) -> None: result, matches_result( failures=contains_exactly( - isTuple( # type: ignore[arg-type] + isTuple( equal_to(case), isFailure( # AssertionError is the type raised by TestCase.fail @@ -172,8 +172,8 @@ def test_runSkip(self) -> None: assert_that( result, matches_result( - skips=contains_exactly( # type: ignore[arg-type] - isTuple( # type: ignore[arg-type] + skips=contains_exactly( + isTuple( equal_to(case), equal_to("pyunit skip"), ), @@ -190,8 +190,8 @@ def test_runUnexpectedSuccesses(self) -> None: assert_that( result, matches_result( - unexpectedSuccesses=contains_exactly( # type: ignore[arg-type] - isTuple( # type: ignore[arg-type] + unexpectedSuccesses=contains_exactly( + isTuple( equal_to(case), equal_to("todo7"), ), From 9eab861b1cedc4442ffbf763919b7da4903d2bf2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Aug 2022 11:50:23 -0400 Subject: [PATCH 10/31] restore more of the original intent of this test make sure stopTest is called only once and with the right argument --- src/twisted/trial/_dist/test/test_worker.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index 8e638c08f2e..e49455635ce 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -216,15 +216,14 @@ def test_stopAfterRun(self) -> None: L{LocalWorkerAMP.run} calls C{stopTest} on its test result once the C{Run} commands has succeeded. """ - + stopped = [] class StopTestResult(TestResult): - _stopped = False - def stopTest(self, test: PyUnitTestCase) -> None: - self._stopped = True + stopped.append(test) - result = self.workerRunTest(pyunitcases.PyUnitTest("test_pass"), StopTestResult) - assert_that(result, has_properties(_stopped=equal_to(True))) + case = pyunitcases.PyUnitTest("test_pass") + result = self.workerRunTest(case, StopTestResult) + assert_that(stopped, equal_to([case])) class SpyDataLocalWorkerAMP(LocalWorkerAMP): From 6deb9f1bd6a237f3792ecc7e39f5e6d01f457fba Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Aug 2022 12:05:24 -0400 Subject: [PATCH 11/31] flatten the matcher in test_runError to see if it helps readability --- src/twisted/trial/_dist/test/test_worker.py | 32 +++++++++------------ 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index e49455635ce..f70435aecb4 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -12,7 +12,7 @@ from zope.interface.verify import verifyObject -from hamcrest import assert_that, contains_exactly, equal_to, has_item, has_properties +from hamcrest import assert_that, contains_exactly, equal_to, has_item, has_length from twisted.internet.defer import fail from twisted.internet.error import ProcessDone @@ -120,24 +120,17 @@ def test_runError(self) -> None: """ Run a test, and encounter an error. """ - case = pyunitcases.PyUnitTest("test_error") - result = self.workerRunTest(case) + expectedCase = pyunitcases.PyUnitTest("test_error") + result = self.workerRunTest(expectedCase) + assert_that(result, matches_result(errors=has_length(1))) + [(actualCase, failure)] = result.errors + assert_that(expectedCase, equal_to(actualCase)) assert_that( - result, - matches_result( - # Failures don't compare nicely so unpack the result and peek inside. - errors=contains_exactly( - isTuple( - equal_to(case), - isFailure( - type=equal_to(Exception), - value=equal_to(WorkerException("pyunit error")), - frames=has_item( # type: ignore[arg-type] - similarFrame("test_error", "pyunitcases.py") # type: ignore[arg-type] - ), - ), - ), - ), + failure, + isFailure( + type=equal_to(Exception), + value=equal_to(WorkerException("pyunit error")), + frames=has_item(similarFrame("test_error", "pyunitcases.py")), # type: ignore[arg-type] ), ) @@ -217,12 +210,13 @@ def test_stopAfterRun(self) -> None: C{Run} commands has succeeded. """ stopped = [] + class StopTestResult(TestResult): def stopTest(self, test: PyUnitTestCase) -> None: stopped.append(test) case = pyunitcases.PyUnitTest("test_pass") - result = self.workerRunTest(case, StopTestResult) + self.workerRunTest(case, StopTestResult) assert_that(stopped, equal_to([case])) From 53471a6660cff71090f27864917fe784e60d7e90 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 12:35:48 -0400 Subject: [PATCH 12/31] Flatten the nested matches into several simpler matches This also removes the need for isTuple --- src/twisted/trial/_dist/test/matchers.py | 11 ---- src/twisted/trial/_dist/test/test_matchers.py | 66 +------------------ src/twisted/trial/_dist/test/test_worker.py | 64 +++++++----------- 3 files changed, 24 insertions(+), 117 deletions(-) diff --git a/src/twisted/trial/_dist/test/matchers.py b/src/twisted/trial/_dist/test/matchers.py index 19ffb8a0055..d783fec8b24 100644 --- a/src/twisted/trial/_dist/test/matchers.py +++ b/src/twisted/trial/_dist/test/matchers.py @@ -190,14 +190,3 @@ def similarFrame( has_length(0), # type: ignore[arg-type] has_length(0), # type: ignore[arg-type] ) - - -def isTuple(*matchers: Matcher[object]) -> Matcher[object]: - """ - Match tuples for which the elements are matched by corresponding - elements of a tuple of matchers. - """ - return AllOf( - instance_of(tuple), - contains_exactly(*matchers), # type: ignore[arg-type] - ) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index ea4d1986728..f010710579e 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -22,7 +22,7 @@ from twisted.python.failure import Failure from twisted.trial.unittest import SynchronousTestCase -from .matchers import HasSum, IsSequenceOf, S, isFailure, isTuple, similarFrame +from .matchers import HasSum, IsSequenceOf, S, isFailure, similarFrame Summer = Callable[[Sequence[S]], S] concatInt = sum @@ -136,70 +136,6 @@ def test_mismatches(self, numBefore: int, numAfter: int) -> None: ) -class IsTupleTests(SynchronousTestCase): - """ - Tests for L{isTuple}. - """ - - @given(lists(integers(), min_size=0, max_size=10)) - def test_matches(self, elements: List[int]) -> None: - """ - L{isTuple} matches tuples if they have the same number of elements - as the number of matchers given and each element is matched by the - corresponding matcher. - - :param elements: The elements with which to populate the tuple to - attempt to match with L{isTuple}. - """ - matcher = isTuple(*(equal_to(e) for e in elements)) - actualDescription = StringDescription() - assert_that(matcher.matches(tuple(elements), actualDescription), equal_to(True)) - assert_that(str(actualDescription), equal_to("")) - - @given( - lists(integers(), min_size=0, max_size=10), - integers(), - lists(integers(), min_size=0, max_size=10), - ) - def test_mismatch(self, before: List[int], mismatch: int, after: List[int]) -> None: - """ - L{isTuple} does not match if any element is not matched. - - :param before: For the tuple to match, elements leading up to an - expected mismatching element. - - :param mismatch: An element expected to mismatch. - - :param after: For the tuple to match, elements following an expected - mismatching element. - """ - matchers = [equal_to(e) for e in before] - matchers.append(not_(anything())) - matchers = [equal_to(e) for e in after] - matcher = isTuple(*matchers) - - elements = tuple(before) + (mismatch,) + tuple(after) - actualDescription = StringDescription() - assert_that(matcher.matches(elements, actualDescription), equal_to(False)) - - @given( - one_of( - lists(integers(), max_size=2), - text(max_size=2), - binary(max_size=2), - integers(), - ), - ) - def test_mismatchOtherType(self, mismatch: object) -> None: - """ - L{isTuple} does not match non-tuple values. - - :param mismatch: A value of a type other than tuple. - """ - matcher = isTuple(anything()) - assert_that(matcher.matches(mismatch), equal_to(False)) - - class IsFailureTests(SynchronousTestCase): """ Tests for L{isFailure}. diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index f70435aecb4..c2fb52480de 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -32,7 +32,7 @@ from twisted.trial.reporter import TestResult from twisted.trial.test import pyunitcases, skipping from twisted.trial.unittest import TestCase, makeTodo -from .matchers import isFailure, isTuple, matches_result, similarFrame +from .matchers import isFailure, matches_result, similarFrame class WorkerProtocolTests(TestCase): @@ -138,21 +138,17 @@ def test_runFailure(self) -> None: """ Run a test, and fail. """ - case = pyunitcases.PyUnitTest("test_fail") - result = self.workerRunTest(case) + expectedCase = pyunitcases.PyUnitTest("test_fail") + result = self.workerRunTest(expectedCase) + assert_that(result, matches_result(failures=has_length(1))) + [(actualCase, failure)] = result.failures + assert_that(expectedCase, equal_to(actualCase)) assert_that( - result, - matches_result( - failures=contains_exactly( - isTuple( - equal_to(case), - isFailure( - # AssertionError is the type raised by TestCase.fail - type=equal_to(AssertionError), - value=equal_to(WorkerException("pyunit failure")), - ), - ), - ), + failure, + isFailure( + # AssertionError is the type raised by TestCase.fail + type=equal_to(AssertionError), + value=equal_to(WorkerException("pyunit failure")), ), ) @@ -160,37 +156,23 @@ def test_runSkip(self) -> None: """ Run a test, but skip it. """ - case = pyunitcases.PyUnitTest("test_skip") - result = self.workerRunTest(case) - assert_that( - result, - matches_result( - skips=contains_exactly( - isTuple( - equal_to(case), - equal_to("pyunit skip"), - ), - ), - ), - ) + expectedCase = pyunitcases.PyUnitTest("test_skip") + result = self.workerRunTest(expectedCase) + assert_that(result, matches_result(skips=has_length(1))) + [(actualCase, skip)] = result.skips + assert_that(expectedCase, equal_to(actualCase)) + assert_that(skip, equal_to("pyunit skip")) def test_runUnexpectedSuccesses(self) -> None: """ Run a test, and succeed unexpectedly. """ - case = skipping.SynchronousStrictTodo("test_todo7") - result = self.workerRunTest(case) - assert_that( - result, - matches_result( - unexpectedSuccesses=contains_exactly( - isTuple( - equal_to(case), - equal_to("todo7"), - ), - ), - ), - ) + expectedCase = skipping.SynchronousStrictTodo("test_todo7") + result = self.workerRunTest(expectedCase) + assert_that(result, matches_result(unexpectedSuccesses=has_length(1))) + [(actualCase, unexpectedSuccess)] = result.unexpectedSuccesses + assert_that(expectedCase, equal_to(actualCase)) + assert_that(unexpectedSuccess, equal_to("todo7")) def test_testWrite(self) -> None: """ From 6412a670e515b13bccbb317424304851733bff86 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 12:56:42 -0400 Subject: [PATCH 13/31] remove unused imports --- src/twisted/trial/_dist/test/test_matchers.py | 2 +- src/twisted/trial/_dist/test/test_worker.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_matchers.py b/src/twisted/trial/_dist/test/test_matchers.py index f010710579e..35ac7070940 100644 --- a/src/twisted/trial/_dist/test/test_matchers.py +++ b/src/twisted/trial/_dist/test/test_matchers.py @@ -2,7 +2,7 @@ Tests for L{twisted.trial._dist.test.matchers}. """ -from typing import Callable, List, Sequence, Tuple, Type +from typing import Callable, Sequence, Tuple, Type from hamcrest import anything, assert_that, contains, contains_string, equal_to, not_ from hamcrest.core.matcher import Matcher diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index c2fb52480de..29b9052c98f 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -12,7 +12,7 @@ from zope.interface.verify import verifyObject -from hamcrest import assert_that, contains_exactly, equal_to, has_item, has_length +from hamcrest import assert_that, equal_to, has_item, has_length from twisted.internet.defer import fail from twisted.internet.error import ProcessDone From aff51189c14aa99ca00c386f82932ffccd3f8e28 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 12:56:50 -0400 Subject: [PATCH 14/31] simplify one more hamcrest usage --- src/twisted/trial/_dist/test/test_worker.py | 22 +++++++++------------ 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/twisted/trial/_dist/test/test_worker.py b/src/twisted/trial/_dist/test/test_worker.py index 29b9052c98f..19e520d713d 100644 --- a/src/twisted/trial/_dist/test/test_worker.py +++ b/src/twisted/trial/_dist/test/test_worker.py @@ -102,19 +102,15 @@ def test_runExpectedFailure(self) -> None: """ Run a test, and fail expectedly. """ - case = skipping.SynchronousStrictTodo("test_todo1") - result = self.workerRunTest(case) - assert_that( - result, - matches_result( - expectedFailures=equal_to( - [ - # Match the strings used in the test we ran. - (case, "expected failure", makeTodo("todo1")), - ] - ) - ), - ) + expectedCase = skipping.SynchronousStrictTodo("test_todo1") + result = self.workerRunTest(expectedCase) + assert_that(result, matches_result(expectedFailures=has_length(1))) + [(actualCase, exceptionMessage, todoReason)] = result.expectedFailures + assert_that(actualCase, equal_to(expectedCase)) + + # Match the strings used in the test we ran. + assert_that(exceptionMessage, equal_to("expected failure")) + assert_that(todoReason, equal_to(makeTodo("todo1"))) def test_runError(self) -> None: """ From 798952fa75a01f2a6b0565f46c1863e224639de2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 12:57:19 -0400 Subject: [PATCH 15/31] type annotate `makeTodo` --- src/twisted/trial/_synctest.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/twisted/trial/_synctest.py b/src/twisted/trial/_synctest.py index 77f14422d20..85d7f32971c 100644 --- a/src/twisted/trial/_synctest.py +++ b/src/twisted/trial/_synctest.py @@ -82,7 +82,11 @@ def expected(self, failure): return False -def makeTodo(value): +def makeTodo( + value: Union[ + str, tuple[Union[Type[BaseException], Iterable[Type[BaseException]]], str] + ] +) -> Todo: """ Return a L{Todo} object built from C{value}. @@ -99,11 +103,11 @@ def makeTodo(value): return Todo(reason=value) if isinstance(value, tuple): errors, reason = value - try: - errors = list(errors) - except TypeError: - errors = [errors] - return Todo(reason=reason, errors=errors) + if isinstance(errors, type): + iterableErrors: Iterable[Type[BaseException]] = [errors] + else: + iterableErrors = errors + return Todo(reason=reason, errors=iterableErrors) class _Warning: From a45afc44d333dc8225efd8a031414fec609c6ab8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 13:03:27 -0400 Subject: [PATCH 16/31] Force the correct types for trial's TestResult They disagree with the inherited type which makes TestResult a very poorly behaved subclass. But, so it is. --- src/twisted/trial/reporter.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/twisted/trial/reporter.py b/src/twisted/trial/reporter.py index 7e7d672a84b..04bd44db598 100644 --- a/src/twisted/trial/reporter.py +++ b/src/twisted/trial/reporter.py @@ -15,6 +15,7 @@ import unittest as pyunit import warnings from collections import OrderedDict +from typing import TYPE_CHECKING, List, Tuple from zope.interface import implementer @@ -24,13 +25,16 @@ from twisted.python.util import untilConcludes from twisted.trial import itrial, util +if TYPE_CHECKING: + from ._synctest import Todo + try: from subunit import TestProtocolClient # type: ignore[import] except ImportError: TestProtocolClient = None -def _makeTodo(value): +def _makeTodo(value: str) -> "Todo": """ Return a L{Todo} object built from C{value}. @@ -81,6 +85,11 @@ class TestResult(pyunit.TestResult): # Used when no todo provided to addExpectedFailure or addUnexpectedSuccess. _DEFAULT_TODO = "Test expected to fail" + skips: List[Tuple[itrial.ITestCase, str]] + expectedFailures: List[Tuple[itrial.ITestCase, str, "Todo"]] # type: ignore[assignment] + unexpectedSuccesses: List[Tuple[itrial.ITestCase, str]] # type: ignore[assignment] + successes: int + def __init__(self): super().__init__() self.skips = [] From 416afd06600adcd945046605b98e70770cd2bb79 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 29 Aug 2022 13:46:08 -0400 Subject: [PATCH 17/31] Old Python compat --- src/twisted/trial/_synctest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/twisted/trial/_synctest.py b/src/twisted/trial/_synctest.py index 85d7f32971c..dcd091f7258 100644 --- a/src/twisted/trial/_synctest.py +++ b/src/twisted/trial/_synctest.py @@ -84,7 +84,7 @@ def expected(self, failure): def makeTodo( value: Union[ - str, tuple[Union[Type[BaseException], Iterable[Type[BaseException]]], str] + str, Tuple[Union[Type[BaseException], Iterable[Type[BaseException]]], str] ] ) -> Todo: """ From a78ee195ae00133442e393684b23ff47181436be Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 08:44:15 -0400 Subject: [PATCH 18/31] Turn on remote debugging --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 5e463f8a9ed..514598a0a50 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -39,7 +39,7 @@ env: # Set to 'yes' to open a tunnel to GitHub's VMs through tmate on failures. # You can also trigger it via manual workflow trigger. # https://github.com/mxschmitt/action-tmate#manually-triggered-debug - TMATE_DEBUG: 'no' + TMATE_DEBUG: 'yes' # The default values in the job generated by the matrix. DEFAULT_PYTHON_VERSION: '3.10' From 057c67e1aa77953233fe4acdb4dcba0a6920306f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 14:54:37 -0400 Subject: [PATCH 19/31] do tmate first --- .github/workflows/test.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 514598a0a50..12a3c4576b8 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -215,10 +215,6 @@ jobs: gir1.2-gtk-3.0 \ xvfb - - name: Test - run: | - ${{ matrix.tox-wrapper }} tox ${{ matrix.trial-target }} - # If one of the above steps fails, fire up tmate for remote debugging. # This is fired for manual trigger or via the environment variable. - name: Tmate debug session @@ -227,6 +223,10 @@ jobs: with: limit-access-to-actor: true + - name: Test + run: | + ${{ matrix.tox-wrapper }} tox ${{ matrix.trial-target }} + - name: Prepare coverage if: ${{ !cancelled() && !matrix.skip-coverage }} run: | From 05077e25aa36fe5cc0deb72ae27b2c763da0eb68 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 14:59:29 -0400 Subject: [PATCH 20/31] some debug junk --- src/twisted/conch/manhole.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index f26e5de974c..3d455d44ce8 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -149,7 +149,10 @@ def _ebDisplayDeferred(self, failure, k, obj): def write(self, data, isAsync=None, **kwargs): isAsync = _get_async_param(isAsync, **kwargs) - self.handler.addOutput(data, isAsync) + sys.__stdout__.flush() + sys.__stderr__.write(f"write() handling {data!r}") + sys.__stderr__.flush() + self.handler.addOutput(data , isAsync) CTRL_C = b"\x03" From 0af88134c40f7b95e5edf32954ddbfecc51760b9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 19:02:54 +0000 Subject: [PATCH 21/31] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/twisted/conch/manhole.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index 3d455d44ce8..5b858255157 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -152,7 +152,7 @@ def write(self, data, isAsync=None, **kwargs): sys.__stdout__.flush() sys.__stderr__.write(f"write() handling {data!r}") sys.__stderr__.flush() - self.handler.addOutput(data , isAsync) + self.handler.addOutput(data, isAsync) CTRL_C = b"\x03" From 495a662e1d2a86e6898007cdc39514e97eadabc1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 15:23:13 -0400 Subject: [PATCH 22/31] don't twiddle stdout, it makes debugging hard the broken test doesn't try to write to sys.stdout so it doesn't matter (I hope!!) --- src/twisted/conch/manhole.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index 3d455d44ce8..e0f47797352 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -106,11 +106,12 @@ def push(self, line): def runcode(self, *a, **kw): orighook, sys.displayhook = sys.displayhook, self.displayhook try: - origout, sys.stdout = sys.stdout, FileWrapper(self.handler) + # origout, sys.stdout = sys.stdout, FileWrapper(self.handler) try: code.InteractiveInterpreter.runcode(self, *a, **kw) finally: - sys.stdout = origout + pass + # sys.stdout = origout finally: sys.displayhook = orighook From dd1482c9df75d356c3c7a14e05eb5929d7b49be1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 16:24:09 -0400 Subject: [PATCH 23/31] breaks other tests, hampers reproduction --- src/twisted/conch/manhole.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index f38cef03973..5b858255157 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -106,12 +106,11 @@ def push(self, line): def runcode(self, *a, **kw): orighook, sys.displayhook = sys.displayhook, self.displayhook try: - # origout, sys.stdout = sys.stdout, FileWrapper(self.handler) + origout, sys.stdout = sys.stdout, FileWrapper(self.handler) try: code.InteractiveInterpreter.runcode(self, *a, **kw) finally: - pass - # sys.stdout = origout + sys.stdout = origout finally: sys.displayhook = orighook From 2ab4d64e71abf32c154392222c0d1a5b0ef73fb1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 16:24:22 -0400 Subject: [PATCH 24/31] better failure messages --- src/twisted/conch/test/test_recvline.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/twisted/conch/test/test_recvline.py b/src/twisted/conch/test/test_recvline.py index 03ac4659889..3fd0157e2d2 100644 --- a/src/twisted/conch/test/test_recvline.py +++ b/src/twisted/conch/test/test_recvline.py @@ -473,15 +473,7 @@ class _BaseMixin: def _assertBuffer(self, lines): receivedLines = self.recvlineClient.__bytes__().splitlines() expectedLines = lines + ([b""] * (self.HEIGHT - len(lines) - 1)) - self.assertEqual(len(receivedLines), len(expectedLines)) - for i in range(len(receivedLines)): - self.assertEqual( - receivedLines[i], - expectedLines[i], - b"".join(receivedLines[max(0, i - 1) : i + 1]) - + b" != " - + b"".join(expectedLines[max(0, i - 1) : i + 1]), - ) + self.assertEqual(receivedLines, expectedLines) def _trivialTest(self, inputLine, output): done = self.recvlineClient.expect(b"done") From d4b5d21681f038ef40269040da5102683bf70b2d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 16:25:47 -0400 Subject: [PATCH 25/31] let tests run first so tox initializes the environment, but end the test run early --- .github/workflows/test.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 12a3c4576b8..a54be6b65e0 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -127,7 +127,7 @@ jobs: # Distributed trial is not yet suported on Windows so we overwrite # the default trial-args to remove concurrent runs and to # select a reactor. - trial-args: '--reactor=select' + trial-args: '--reactor=select -x' # Windows, newest Python supported version with iocp reactor. - python-version: '3.10' @@ -135,7 +135,7 @@ jobs: tox-env: 'alldeps-withcov-windows' job-name: 'win-default-tests-iocp' # Distributed trial is not yet suported on Windows. - trial-args: '--reactor=iocp' + trial-args: '--reactor=iocp -x' # On PYPY coverage is very slow (5min vs 30min) as there is no C # extension. @@ -215,6 +215,10 @@ jobs: gir1.2-gtk-3.0 \ xvfb + - name: Test + run: | + ${{ matrix.tox-wrapper }} tox ${{ matrix.trial-target }} + # If one of the above steps fails, fire up tmate for remote debugging. # This is fired for manual trigger or via the environment variable. - name: Tmate debug session @@ -223,10 +227,6 @@ jobs: with: limit-access-to-actor: true - - name: Test - run: | - ${{ matrix.tox-wrapper }} tox ${{ matrix.trial-target }} - - name: Prepare coverage if: ${{ !cancelled() && !matrix.skip-coverage }} run: | From e0f874275d0fc8e89177fa24d097820843cf53f0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 16:36:52 -0400 Subject: [PATCH 26/31] dump more debug info --- src/twisted/conch/test/test_manhole.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/twisted/conch/test/test_manhole.py b/src/twisted/conch/test/test_manhole.py index ef07bd24bcb..07ae2701c07 100644 --- a/src/twisted/conch/test/test_manhole.py +++ b/src/twisted/conch/test/test_manhole.py @@ -231,6 +231,14 @@ def test_Exception(self): """ Evaluate raising an exception. """ + import sys + print(f"\n\nexcepthook: {sys.excepthook}") + print(f"__excepthook__: {sys.__excepthook__}") + print(f"sys.stdout: {sys.stdout}") + print(f"sys.__stdout__: {sys.__stdout__}") + print(f"sys.stderr: {sys.stderr}") + print(f"sys.__stderr__: {sys.__stderr__}\n\n") + done = self.recvlineClient.expect(b"done") self._testwrite(b"raise Exception('foo bar baz')\n" b"done") From 3a3f278778aed626a539d2a4ee3a5a754512909d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 1 Sep 2022 16:37:10 -0400 Subject: [PATCH 27/31] not really usable --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a54be6b65e0..086588f6704 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -39,7 +39,7 @@ env: # Set to 'yes' to open a tunnel to GitHub's VMs through tmate on failures. # You can also trigger it via manual workflow trigger. # https://github.com/mxschmitt/action-tmate#manually-triggered-debug - TMATE_DEBUG: 'yes' + TMATE_DEBUG: 'no' # The default values in the job generated by the matrix. DEFAULT_PYTHON_VERSION: '3.10' From 7a2775dd38cccce48dc15a658e3be059d1e8cb26 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 20:40:24 +0000 Subject: [PATCH 28/31] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/twisted/conch/test/test_manhole.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/twisted/conch/test/test_manhole.py b/src/twisted/conch/test/test_manhole.py index 07ae2701c07..9ab4896866c 100644 --- a/src/twisted/conch/test/test_manhole.py +++ b/src/twisted/conch/test/test_manhole.py @@ -232,6 +232,7 @@ def test_Exception(self): Evaluate raising an exception. """ import sys + print(f"\n\nexcepthook: {sys.excepthook}") print(f"__excepthook__: {sys.__excepthook__}") print(f"sys.stdout: {sys.stdout}") From b00c683badbb2cec56faf958eeb7b2d9cc7c9d75 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 6 Sep 2022 08:52:01 -0400 Subject: [PATCH 29/31] maybe eek out a bit more info --- .github/workflows/test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 086588f6704..faed80f932f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -127,7 +127,7 @@ jobs: # Distributed trial is not yet suported on Windows so we overwrite # the default trial-args to remove concurrent runs and to # select a reactor. - trial-args: '--reactor=select -x' + trial-args: '--reactor=select --debug --nopm -x' # Windows, newest Python supported version with iocp reactor. - python-version: '3.10' @@ -135,7 +135,7 @@ jobs: tox-env: 'alldeps-withcov-windows' job-name: 'win-default-tests-iocp' # Distributed trial is not yet suported on Windows. - trial-args: '--reactor=iocp -x' + trial-args: '--reactor=iocp --debug --nopm -x' # On PYPY coverage is very slow (5min vs 30min) as there is no C # extension. From 804ceefe2f28fd6ee974be10395beaa2c7e012c2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 6 Sep 2022 09:10:13 -0400 Subject: [PATCH 30/31] debug stuff interferes with test logic! rad --- src/twisted/conch/manhole.py | 6 +++--- src/twisted/conch/test/test_manhole.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index 5b858255157..3d539029b16 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -149,9 +149,9 @@ def _ebDisplayDeferred(self, failure, k, obj): def write(self, data, isAsync=None, **kwargs): isAsync = _get_async_param(isAsync, **kwargs) - sys.__stdout__.flush() - sys.__stderr__.write(f"write() handling {data!r}") - sys.__stderr__.flush() + # sys.__stdout__.flush() + # sys.__stderr__.write(f"write() handling {data!r}") + # sys.__stderr__.flush() self.handler.addOutput(data, isAsync) diff --git a/src/twisted/conch/test/test_manhole.py b/src/twisted/conch/test/test_manhole.py index 9ab4896866c..cd12681f37f 100644 --- a/src/twisted/conch/test/test_manhole.py +++ b/src/twisted/conch/test/test_manhole.py @@ -231,14 +231,14 @@ def test_Exception(self): """ Evaluate raising an exception. """ - import sys - - print(f"\n\nexcepthook: {sys.excepthook}") - print(f"__excepthook__: {sys.__excepthook__}") - print(f"sys.stdout: {sys.stdout}") - print(f"sys.__stdout__: {sys.__stdout__}") - print(f"sys.stderr: {sys.stderr}") - print(f"sys.__stderr__: {sys.__stderr__}\n\n") + # import sys + + # print(f"\n\nexcepthook: {sys.excepthook}") + # print(f"__excepthook__: {sys.__excepthook__}") + # print(f"sys.stdout: {sys.stdout}") + # print(f"sys.__stdout__: {sys.__stdout__}") + # print(f"sys.stderr: {sys.stderr}") + # print(f"sys.__stderr__: {sys.__stderr__}\n\n") done = self.recvlineClient.expect(b"done") From d61258165a632adbf71df85693d45931a864cce8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 6 Sep 2022 09:23:18 -0400 Subject: [PATCH 31/31] remove debug stuff the failure went away on CI. will it still be gone in the next run? who can say --- .github/workflows/test.yaml | 4 ++-- src/twisted/conch/manhole.py | 3 --- src/twisted/conch/test/test_manhole.py | 9 --------- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index faed80f932f..5e463f8a9ed 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -127,7 +127,7 @@ jobs: # Distributed trial is not yet suported on Windows so we overwrite # the default trial-args to remove concurrent runs and to # select a reactor. - trial-args: '--reactor=select --debug --nopm -x' + trial-args: '--reactor=select' # Windows, newest Python supported version with iocp reactor. - python-version: '3.10' @@ -135,7 +135,7 @@ jobs: tox-env: 'alldeps-withcov-windows' job-name: 'win-default-tests-iocp' # Distributed trial is not yet suported on Windows. - trial-args: '--reactor=iocp --debug --nopm -x' + trial-args: '--reactor=iocp' # On PYPY coverage is very slow (5min vs 30min) as there is no C # extension. diff --git a/src/twisted/conch/manhole.py b/src/twisted/conch/manhole.py index 3d539029b16..f26e5de974c 100644 --- a/src/twisted/conch/manhole.py +++ b/src/twisted/conch/manhole.py @@ -149,9 +149,6 @@ def _ebDisplayDeferred(self, failure, k, obj): def write(self, data, isAsync=None, **kwargs): isAsync = _get_async_param(isAsync, **kwargs) - # sys.__stdout__.flush() - # sys.__stderr__.write(f"write() handling {data!r}") - # sys.__stderr__.flush() self.handler.addOutput(data, isAsync) diff --git a/src/twisted/conch/test/test_manhole.py b/src/twisted/conch/test/test_manhole.py index cd12681f37f..ef07bd24bcb 100644 --- a/src/twisted/conch/test/test_manhole.py +++ b/src/twisted/conch/test/test_manhole.py @@ -231,15 +231,6 @@ def test_Exception(self): """ Evaluate raising an exception. """ - # import sys - - # print(f"\n\nexcepthook: {sys.excepthook}") - # print(f"__excepthook__: {sys.__excepthook__}") - # print(f"sys.stdout: {sys.stdout}") - # print(f"sys.__stdout__: {sys.__stdout__}") - # print(f"sys.stderr: {sys.stderr}") - # print(f"sys.__stderr__: {sys.__stderr__}\n\n") - done = self.recvlineClient.expect(b"done") self._testwrite(b"raise Exception('foo bar baz')\n" b"done")