treq.auth omnibus

[twm]

• Rework the treq.test.test_auth to use a new treq._recorder.recorder() utility rather than mock.
• Closes add_auth() agent doesn't implement IAgent #312: Mark _RequestHeaderSettingAgent as an IAgent implementer.
• Closes Document treq.auth #313: Document treq.auth — add docstrings and add to the API page.
• Closes treq.auth.add_basic_auth agent mutates passed Headers #314: Don't mutate Headers passed to _RequestHeaderSettingAgent
• Closes utf8 encode basic auth username/password #268: UTF-8 encode basic auth username and password. Also allow bytes as an escape hatch.

[adiroiban]

Great improvement. Thanks!

Only minor comments.

And this PR can be merged as is.

---

Happy to see mock.Mock gone :)

For my tests, I stopped using spies that record and for which you assert the behaviour later,
and instead I am using and ReplayAgentBacked similar to the angularjs $httpBackend

You pass a list of expected URLs and request data, but you also able to return a response.
In this way, you can test chained agent interactions, without touching the network.

For example, I am using for SAML login testing in which more than 1 requests are made...and the response of the first request is used in the second request.

[adiroiban]

minor comment

maybe rename it as get_http_agent_spy and allow to pass any list?

Right now, having the list managed outside of recorder() makes no difference and maybe just keep it as get_http_agent_spy and return a tupple.

Suggested change

	def recorder():
	def get_http_agent_spy(accumulator_callable):

[twm]

I like the "agent spy" terminology, but I definitely want a function like recorder() that is low lines-of-code to use in a test. If using this API takes more than one or two lines of code folks will move it to setUp which then makes the tests more verbose and less amenable to local reasoning.

[adiroiban]

you write one, read many times :) ... and with a good name, you read the name and you don't have to search for the documentation of that name :)

and then, I guess that all developers have auto completion :)

And if someones adds stuff to setUp just complain during review.

It's your call.
I don't think that is fair to make the code harder to read just because some people are lazy :p

[twm]

I renamed recorder to agent_spy and _RequestRecordAgent to _AgentSpy. The class could be made public to provide the bring-your-own-accumulator API, or another wrapper function added. Probably the latter is best to prevent subclassing.

[adiroiban]

looks good. I think that this test helper would also be very useful in core Twisted HTTP as it might be used by other tests in which twisted.web.iweb.IAgent is used.

If you have time for a review, I could try to add it to Twisted, but I need a review buddy.
If if you want to add it to twisted, I would be happy to review it.

[twm]

I'd be happy to see it added there. Indeed it woudn't surprise me if something similar already exists somewhere in twisted.web.test. I'd be happy to review if you port it over, or I can do so.

I filed https://twistedmatrix.com/trac/ticket/10072 (with AgentSpy as the name) to add this to Twisted. Please self-assign the ticket when you start working on it; I'll do the same if I get to it first.

[twm]

Happy to see mock.Mock gone :)

More of this to come!

For my tests, I stopped using spies that record and for which you assert the behaviour later,
and instead I am using and ReplayAgentBacked similar to the angularjs $httpBackend

You pass a list of expected URLs and request data, but you also able to return a response.
In this way, you can test chained agent interactions, without touching the network.

For example, I am using for SAML login testing in which more than 1 requests are made...and the response of the first request is used in the second request.

treq.testing, specifically RequestTraversalAgent, is exactly this! I'm a big fan of this style. It's actually the part of treq I use most, though it has some pretty rough edges. I didn't use it in this case for two reasons:

• treq.testing itself is a big hunk of code, far more complex than the treq.auth implementation itself, so it seemed risky.
• I plan to revise the treq.testing API once treq drops Python 2.7 support, so I didn't want to add an internal dependency on its current API.

[adiroiban]

I think that the spy is OK for unit tests and treq.testing can be used "network-free" integration tests.

[twm]

Thank you very much for the review and feedback, @adiroiban!