Allow overriding ClientSession class in TestClient

[cecton]

What do these changes do?

It adds a keyword argument to the TestClient class to override the default ClientSession used.

Are there changes in behavior for the user?

None.

Related issue number

#2032 #2074

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the changes folder
  • name it <issue_id>.<type> for example (588.bug)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[asvetlov]

Sorry, I still don't understand why do you need to use custom client session.
I believe inheritance from ClientSession is discouraged (while not prohibited)

[codecov-io]

Codecov Report

Merging #2142 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2142      +/-   ##
==========================================
+ Coverage   97.09%   97.09%   +<.01%     
==========================================
  Files          38       38              
  Lines        7740     7741       +1     
  Branches     1351     1351              
==========================================
+ Hits         7515     7516       +1     
  Misses        101      101              
  Partials      124      124

Impacted Files	Coverage Δ
aiohttp/test_utils.py	98.61% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44edff8...0ffd9dd. Read the comment docs.

[cecton]

Really? I thought it was common usage. That's how docker-py works https://github.com/docker/docker-py/blob/master/docker/api/client.py#L44 I did the same for aiosparql: https://github.com/aio-libs/aiosparql/blob/master/aiosparql/client.py#L81

(In case of aiosparql I just did it because then I can use the async with automatically, otherwise I have to implement all the methods for it.)

[asvetlov]

@cecton looks like I've lost control onto aiohttp classes usage (again and again).
My initial thought about ClientSession is the session should be sealed, if anybody need to extend it -- please make a wrapper with overriding session.get, session.post and others.
Embedding is better than inheritance etc.

But I see people inherit from session widely.
Well, I could live with it.

The PR is good but please add a check for missed RuntimeError coverage.
After that feel free to merge.

[cecton]

@asvetlov I can hardly test at that level because it's testing the test. But I will think of it... if you have any idea... :)

[cecton]

The example 2 here could help http://www.programcreek.com/python/example/7163/unittest.expectedFailure

Will do that

[asvetlov]

Well, #pragma: no cover will work too.

[cecton]

@asvetlov there is something fishy with the loop fixture that I will check another time. If I don't use the fixture loop, I can see this in the logs (by using -s of py.test).

============================================== 1 passed in 0.45 seconds ===============================================
Exception ignored in: <bound method BaseEventLoop.__del__ of <_UnixSelectorEventLoop running=False closed=True debug=Fa
lse>>
Traceback (most recent call last):
  File "/usr/lib/python3.5/asyncio/base_events.py", line 431, in __del__
  File "/usr/lib/python3.5/asyncio/unix_events.py", line 58, in close
  File "/usr/lib/python3.5/asyncio/unix_events.py", line 139, in remove_signal_handler
  File "/usr/lib/python3.5/signal.py", line 47, in signal
TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object
py35 runtests: commands[1] | mv .coverage .coverage.py35
_______________________________________________________ summary _______________________________________________________
  py35: commands succeeded
  congratulations :)

Not blocking though... just unclean.

[cecton]

@asvetlov I have the feeling the tests are run 3 times on travis... it takes ages compared to appveyor... something wrong too

[cecton]

@asvetlov If you think one should not inherit from ClientSession, then maybe it's best to not encourage this behavior at all. Maybe it is best that we keep only the second commit which is simply handy and drop the first one. Anyone who really needs that will find their way to hack around to get it done anyway.

[asvetlov]

Yes, the second commit only looks much better

[cecton]

Closed in favor of #2151

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.