Opened 12 years ago

Closed 12 years ago

#2898 enhancement closed fixed (fixed)

Add ITestCase for Trial tests

Reported by: Jonathan Lange Owned by:
Priority: highest Milestone:
Component: trial Keywords:
Cc: therve Branch: branches/itestcase-2898-2
branch-diff, diff-cov, branch-cov, buildbot
Author: jml

Description

Define an interface for tests as used by the test framework and then do pyunit compat with an adapter.

Change History (19)

comment:1 Changed 12 years ago by Jonathan Lange

Branch: branches/itestcase-2898

(In [21751]) Branching to 'itestcase-2898'

comment:2 Changed 12 years ago by Jonathan Lange

Component: coretrial
Keywords: review added
Owner: Glyph deleted
Priority: normalhighest

comment:3 Changed 12 years ago by Jonathan Lange

Keywords: review removed
Owner: set to Jonathan Lange

Doesn't work on Python 2.3.

comment:4 Changed 12 years ago by Jonathan Lange

Keywords: review added

Fixed in Python 2.3.

comment:5 Changed 12 years ago by Jonathan Lange

Owner: Jonathan Lange deleted

comment:6 Changed 12 years ago by therve

Owner: set to therve

comment:7 Changed 12 years ago by therve

Cc: therve added
Keywords: review removed
Owner: changed from therve to Jonathan Lange

Global comments:

  • the new warnings are not tested. The test suite now prints warnings (but not only from these changes, unfortunately). At least, the new warnings emitted should be filtered. Also, the text of the warnings are incomplete, it should explicitly say what is deprecated, and if there is an alternative, point to it.
  • test_doctest.py and test_loader.py seem to contain unrelated changes

In test_pyunitcompat.py:

  • the docstring of test_id is not clear

In runner.py:

  • Why did you remove completely the docstring of DocTestSuite? The fact it's deprecated doesn't mean it should be undocumented :)
  • I don't understand the change in isTestCase. Isn't the goal to use ITestCase more?

In unittest.py:

  • _AdaptedReporter and its methods have no docstring

In itrial.py:

  • Why setUp and tearDown have been removed?
  • it's unfortunate that __call__ is introduced, but that may depends on stdlib unittest.

Also, if you can explain why after deprecating it, we now reuse it, that would be great. Thanks!

comment:8 Changed 12 years ago by Jonathan Lange

(In [21801]) Remove unnecessary test. Refs #2898.

comment:9 Changed 12 years ago by Jonathan Lange

Branch: branches/itestcase-2898branches/itestcase-2898-2

(In [21802]) Branching to 'itestcase-2898-2'

comment:10 Changed 12 years ago by Jonathan Lange

I'll add a more general note before I continue fixing the details.

The goal of this branch is to provide an expected interface for unit tests, and to use this interface to handle incompatibilities. By coincidence, there is a deprecated, stupid interface with the name that we want, that is only used as a way of marking classes that we want to try to load as tests.

This interface must be the public methods and attributes that are used by testing frameworks. In other words, given a test case object, what can I do with it?

For this reason setUp and tearDown are not included. They are part of a separate contract provided by the run() method of many TestCase-style classes.

The interface cannot just define the contract that Trial's test framework relies on, it must also define the contract that other test frameworks can be expected to rely on. For our purposes, "other test frameworks" means Python standard library unittest. Trial tests should be useable in stdlib test suites: that's why we must implement __call__.

So we have an interface that defines exactly what Trial expects from a test case (ITestCase), and we want to support tests that don't yet implement that interface (pyunit.TestCase). We want to do this using an adapter.

That means we can't use ITestCase to find the pyunit test cases. Checking for subclasses of pyunit.TestCase is the simplest thing that is correct.

This is a long explanation. Its complexity is one of the reasons that I'm breaking up my work on introducing test decorators.

I hope this helps.

comment:11 Changed 12 years ago by Jonathan Lange

Keywords: review added
Owner: changed from Jonathan Lange to therve
  • The unrelated changes are there from when I've been experimenting with those files and fixing up the code to be standards compliant. Those changes are staying there.
  • Restored docstring to DocTestSuite.
  • Basically, no one should be using any of the stuff in twisted.trial.runner. It's support code for the testing framework. DocTestSuite was just a technique for Trial to support doctests. Now we are switching to a different, more general technique that doesn't have an analog for DocTestSuite.
  • Docstrings added for _AdaptedReporter.

comment:12 Changed 12 years ago by therve

Owner: therve deleted

Well, some of my previous comments are still true: new warnings are being printed, whereas they should be suppressed, and there is no test for warnings. If you don't care about that, fine, but I'm not able to review that code then.

comment:13 Changed 12 years ago by Jonathan Lange

author: jml

comment:14 Changed 12 years ago by therve

The tests printing new warnings are twisted.trial.test.test_doctest.TestRunners.test_correctCount and twisted.trial.test.test_loader.LoaderTest.test_loadDifferentNames. Although, they may appear somewhere else after these ones are corrected (I think? warnings is so strange).

comment:15 Changed 12 years ago by Jonathan Lange

Warnings fixed.

comment:16 Changed 12 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jonathan Lange

Some questions:

  • Is iterateTests intended to be public?
  • Why does _AdaptedReporter only implement addError and not the other methods which take a test case?
  • suiteVisit and DocTestSuite in runner.py changed to adapt objects by calling ITestCase instead of explicitly instantiating the right adapter class. Is there a deep reason for this, or is it only to avoid duplicating the logic which associates PyUnitTestCase with pyunit.TestCase which is already present in unittest.py (in the registerAdapter calls)? If so, cool. Generally it's nicer to explicitly instantiate an adapter class in code which definitely knows what kind of thing is being adapted, though.

One simple thing:

  • The docstring for ITestCase.failureException might want to say exception class instead of exception

comment:17 Changed 12 years ago by Jonathan Lange

In order:

  • Not really. Renamed to _iterateTests
  • It's all that's needed to make it work. There'll be a branch soon that provides a full implementation.
  • _PyUnitTestAdapter is different from PyUniTestCase. In these cases it's not actually known which classes are being adapted.
  • Docstring updated.

comment:18 Changed 12 years ago by Jonathan Lange

Resolution: fixed
Status: newclosed

(In [21827]) Define ITestCase to be the interface that test frameworks expect of a test.

  • Author: jml
  • Reviewers: therve, exarkun
  • Fixes #2898

ITestCase is re-purposed to define what you can expect from a test object. We use this interface to adapt pyunit tests to things usable in Trial.

In the process we also clarify what's needed for compatibility with doctest and pyunit.

This branch also fixes #1950.

Refs #2739.

comment:19 Changed 9 years ago by <automation>

Owner: Jonathan Lange deleted
Note: See TracTickets for help on using tickets.