Refactor tests, add base TestCase class

Refactor tests, add base TestCase class

[yselivanov, 2014-06-17 00:16:23]

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py
File asyncio/test_utils.py (right):

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode386
asyncio/test_utils.py:386: def set_event_loop(self, loop, cleanup=True):
Could you please make 'cleanup' parameter keyword-only?

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode387
asyncio/test_utils.py:387: events.set_event_loop(None)
I think this line deserves a good comment, that will explain why we test with
event loop set to None in default policy.

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode388
asyncio/test_utils.py:388: if cleanup:
should it be "if cleanup and loop is not None"?

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode392
asyncio/test_utils.py:392: loop = TestLoop(gen)
Should we have a class-level property TestCase.TestLoop, so that it's easy to
test different loops with test case inheritance?

[yselivanov, 2014-06-17 00:16:51]

+1 for change in general

[haypo_gmail, 2014-06-17 00:35:16]

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py
File asyncio/test_utils.py (right):

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode386
asyncio/test_utils.py:386: def set_event_loop(self, loop, cleanup=True):
On 2014/06/17 00:16:23, yselivanov wrote:
> Could you please make 'cleanup' parameter keyword-only?

Yes I can :-) I will do that in my next version.

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode387
asyncio/test_utils.py:387: events.set_event_loop(None)
On 2014/06/17 00:16:23, yselivanov wrote:
> I think this line deserves a good comment, that will explain why we test with
> event loop set to None in default policy.

I found a nice comment in test_subprocess.py:

# ensure that the event loop is passed explicitly in the code

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode388
asyncio/test_utils.py:388: if cleanup:
On 2014/06/17 00:16:23, yselivanov wrote:
> should it be "if cleanup and loop is not None"?

Oh, I wrote this patch just to prepare a new patch which calls
events.set_event_loop(loop) if loop.get_debug() is True:

https://code.google.com/p/tulip/issues/detail?id=176

It means that you should not pass None to set_event_loop(). Call directly
asyncio.set_event_loop(None) if you want to pass None.

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode392
asyncio/test_utils.py:392: loop = TestLoop(gen)
On 2014/06/17 00:16:23, yselivanov wrote:
> Should we have a class-level property TestCase.TestLoop, so that it's easy to
> test different loops with test case inheritance?

Most test cases use TestLoop. The remaining test cases have longer code to
configure exactly an event loop. I'm not interested to factorized this code, it
would just make the code harder to understand, but I don't think that it would
make the code shorter.

Example of the next file:

self.loop = base_events.BaseEventLoop()
self.loop._selector = mock.Mock()
self.set_event_loop(self.loop)

It's just 3 lines, only written once. IMO there is no need to factorize code
here.

[yselivanov, 2014-06-17 00:39:58]

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py
File asyncio/test_utils.py (right):

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode388
asyncio/test_utils.py:388: if cleanup:
> It means that you should not pass None to set_event_loop(). Call directly
> asyncio.set_event_loop(None) if you want to pass None.

I this case I think we can have an assert statement with some explanation, like

assert loop, 'loop should never be None. Call asyncio.set_event_loop(None)
directly'

https://codereview.appspot.com/105270044/diff/1/asyncio/test_utils.py#newcode392
asyncio/test_utils.py:392: loop = TestLoop(gen)
OK ;)