bpo-32972: Unittest: Add a runTestMethod to TestCase (don't merge)

[PetterS]

This allows a future subclass to customize how the test method is run. For
example, this is required in order to create an "AsyncTestCase" that runs
coroutines in an event loop.

This pull request only provides the neccessary change in unittest. Any future class like "AsyncTestCase" will probably live in asyncio.

https://bugs.python.org/issue32972

[1st1]

Since this is documented as something that can be overloaded in subclasses, I think it should be a public API.

[asvetlov]

+1

[PetterS]

Done!

Should it be publicly documented (in unittest.rst) as well?

[asvetlov]

Sure, the runTestMethod should be documented.
I quite unhappy with runTestMethod name. doTest maybe.
Moreover thinking on adding async support I see the patch is not complete: we need overridable methods for setUp()/tearDown() calls. Keep in mind that tear down is actually a bundle of self.tearDown() and running cleanups added by self.addCleanup(...).
That's why I would like to have three overridables: doTest(), doSetUp and doTearDown (or doCleanup.
Does it make sense?

Another question is FunctionTestCase. Should we support async tests for the class?
I think "no" is an easy and satisfactory answer but maybe somebody disagree.

P.S.
@PetterS thank you for moving our discussion into pull request, it's very hard to get the whole vision without looking on concrete code change proposals.

[asvetlov]

Added don't merge to title, I have a feeling that we should careful discuss my questions (and I expect a raising new ones during discussion).

[PetterS]

I am not sure we need doSetUp and doTearDown . I got the impression from the discussion that setUp and tearDown should never need to be async. Thus, only a overridable test method runner would be needed.

As for the name, I have no opinion either way. doTest sounds good to me. Happy to change it unless anyone objects.

[1st1]

The PR looks good, but there's still an open issue: setUp and tearDown shouldn't be async, but we still need a way to have their asyncTearDown and asyncSetUp equivalents.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[PetterS]

Since setUp and tearDown shouldn't be async, I am nore sure that there is much left to do in TestCase at this point. I was thinking that asyncSetup and asyncTearDown can be implemented in the subclass using setUp and tearDown without any special support in TestCase. That way, we don't have to increase the public API of TestCase that much.

I believe that we can implement an AsyncTestCase with what we have in this PR.

[asvetlov]

People assume that setUp() / tearDown() do nothing in default implementation.
It means overridden MyTest.setUp() doesn't call super().setUp() very often.
If AsyncioTestCase will override these methods to call asyncSetUp()/asyncDearDown() this changes game rules: MyTest.setUp() should call parent method.

That's why I propose an alternative: let's support doSetUp and doTearDown methods. By default doSetUp calls self.setUp(), doTearDown calls self.tearDown(). AsyncioTestCase can add custom calls to asyncSetUp() / asyncTearDown() without changing setUp()/tearDown() expectations.

[PetterS]

Yes, that would work, but having both setUp and doSetUp defined and documented for TestCase is not a very nice interface.

Better to put as much of the new logic as possible in the new class. (IMO)

[asvetlov]

In my mind all three do* methods should be documented as internals.
It means they should be used by new test case classes creators (e.g AsyncioTestCase) but not final tests (e.g. class TestMyFancyStuff(AsyncioTestCase):).

Otherwise we have an overload for running test itself only but setup/teardown is rigidly pinned.

See these lines from ./Lib/unittest/case.py:

 with outcome.testPartExecutor(self): self.setUp() if outcome.success: outcome.expecting_failure = expecting_failure with outcome.testPartExecutor(self, isTest=True): testMethod() outcome.expecting_failure = False with outcome.testPartExecutor(self): self.tearDown() self.doCleanups() 

The PR customizes testMethod() execution only. Why setup/teardown is not customized as well?

[PetterS]

@1st1, you requested changes. Is that what you had in mind? Personally, I am not sure that there is much left to do in TestCase at this point.

[1st1]

@asvetlov @PetterS Please let's try to keep the discussion about the API in the issue.

[asvetlov]

That's what I'm talking about: we need three hooks: set-up, run-test and tear-down.