Added get_client and get_server methods to AioHTTPTestCase

[cecton]

This change is needed if we want a clear distinction in the
unittest.TestCase between "app" (the application) and "server" the
actual test server. This will also allow a developer to override these
methods to use their own TestServer and TestClient.

What do these changes do?

1. It transform the method _get_client() of AioHTTPTestCase to a public method (inviting the developer to override it)
2. It adds a method get_server() to AioHTTPTestCase that will, by default, instantiate the default TestServer
3. It replaces the flexible first parameter of TestClient which was app_or_server to server only
4. It removes the now unrelevant parameter host an port of TestClient

Are there changes in behavior for the user?

1. (breaking change) You can no longer return a BaseTestServer instance from get_application. get_application must only return an Application instance.
2. If you want a different TestServer you must override get_server.
3. You can now override get_client method because it is now a public method (you always could but... now it's more obvious that you actually should).
4. (breaking change) _get_client has change of name.

Related issue number

#2032

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."

[codecov-io]

Codecov Report

Merging #2074 into master will decrease coverage by 0.03%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #2074      +/-   ##
==========================================
- Coverage   97.08%   97.04%   -0.04%     
==========================================
  Files          38       38              
  Lines        7689     7692       +3     
  Branches     1344     1342       -2     
==========================================
  Hits         7465     7465              
- Misses        102      103       +1     
- Partials      122      124       +2

Impacted Files	Coverage Δ
aiohttp/test_utils.py	98.92% <100%> (-0.36%)	⬇️
aiohttp/pytest_plugin.py	89.51% <81.81%> (-1.55%)	⬇️

---

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 fb0a781...1064738. Read the comment docs.

[asvetlov]

I'm ok with adding get_client() and get_server() public methods to AioHTTPTestCase.
But prohibiting an application passing to TestClient is too invasive change (you could notice how many tests are touched by this PR).

[cecton]

I think you're right....

My point is mostly for the unittest's TestCase where it makes much more sense and also makes the code simple (if you compare the TestClient class before and after, it's clearly just easier). But I completely ignored the test_ methods.

I'm giving it another try.

[asvetlov]

You proposal for TestClient ctor change makes sense but ship has sailed more than year ago.
Historically the client supported Application only, after that I added TestServer also.
I could live with accepting both app and server. Not very elegant code but handy for usage.

Hmm. We could change test_client fixture and get rid of application in TestClient itself.
It should be relative smooth transition.

[cecton]

Yes that's what I just realized. I need to make the tests in test_test_utils.py files more elegant but it's getting closer.

[cecton]

Actually no I can't make it look better because the purpose of the test_test_utils.py is to test test_utils.py. Since I changed the API, there are changes in those files. Makes sense.

I just made the passing of argument loop more explicit.

@asvetlov do you think this PR looks much better now?

[cecton]

I dropped here the duplicated test on TestServer and RawTestServer. The argument to TestClient must now be a BaseTestServer instance which includes both. Maybe someone would want to make a TestServer not based on TestServer but on BaseTestServer. That also reflects better the first argument of TestClient.

[cecton]

This reproduces the exact behavior than before but a little more strict:

1. __param must be a callable, in the previous code it was the "else" case
2. kwargs is reset just after because we don't want them to be passed to the client or server (identical than before)
3. by extension, the result of the callable must be an instance of Application or BaseTestServer or it would complain explicitly about it

[cecton]

I just added setUpAsync and tearDownAsync. They are very useful in development because you don't need to write synchronous methods for the setUp and tearDown anymore, you can simply override those. (If you want I have made tests cases for those in particular, it instantiate the test case in a test and check that both functions executed.)

That was my last change for this PR, thanks for your time.

[cecton]

@asvetlov this is ready for your review.

[asvetlov]

Thanks!

[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.