Adding an in-memory stub version of treq #96

cyli · 2015-07-07T01:28:43Z

Thanks to @glyph for the suggestion of using the request traversal agent from https://github.com/rackerlabs/mimic, which means that treq actually operates and produces a real response, so we don't have to make a verified fake response, and for help debugging HTTPS and request issues.

Part of addressing #27.

Was going to write the docs and tutorial next if this looks good.

…sure that it has the same interface and will return stubbed responses if given an appropriate resource.

… that we can have a synchronous IBodyProducer instead of something that requires a cooperator.

…quests should have no body.

…responses.

…we want to check if the expected headers is a strict subset of the actual headers. Fix bugs.

…equests and responses.

cyli · 2015-07-07T17:49:13Z

This is failing due to #97

glyph · 2015-07-08T03:27:44Z

Just a quick point of order to track provenance; @cyli is a Rackspace employee, and although this code originates from the APL2-licensed Mimic, the files in question have only been touched by other Rackspace employees and so she is authorized to relicense it as MIT to contribute it upstream to Treq.

glyph · 2015-07-08T03:31:17Z

I just merged master to update the Travis results.

glyph · 2015-07-08T03:44:11Z

treq/client.py

        self._agent = agent
        self._cookiejar = cookiejar or cookiejar_from_dict({})
+        self._bodyproducer = bodyproducer


This attribute should probably be called something more like dataToBodyProducer since it is not itself an IBodyProducer but rather the way that you get one.

codecov-io · 2015-07-08T18:31:37Z

Current coverage is `95.82%`

Merging #96 into master will increase coverage by +0.51% as of b941516

Coverage Diff

@@            master     #96   diff @@
======================================
  Files           17      19     +2
  Stmts         1259    1411   +152
  Branches        97     112    +15
  Methods          0       0       
======================================
+ Hit           1200    1352   +152
  Partial         19      19       
  Missed          40      40

Powered by Codecov

glyph · 2015-07-08T20:19:30Z

I think the codecov report is in fact noticing a problem - there are more 'partial' and 'missed' lines in this branch. That said - who added codecov to this repo?

hynek · 2015-07-08T20:54:00Z

#100

cyli · 2015-07-08T21:14:26Z

There are a couple of missing lines of coverage in testing.py:

abortConnection's call to loseConnection: I guess abortConnection is never called - I'm not sure how to trigger that in the tests - will that work if we just cancel an outstanding request?
SynchronousProducer.stopProducing is only partially covered? I'm not entirely sure how to trigger this.
IStringResponseStubs.get_response_for - I'd expect this to not be covered at all, since it's an interface - I'm not sure why it's partially covered. I guess we should add a # pragma no-cover or something to this line?
the ImportError from trying to import twisted.web.iweb.IAgent is not covered - this is odd, since the tests are running twisted < 13.1.0, which means that there should be an ImportError importing it for those test environments - are the coverage reports from all the different environments merged by codecov? (@hynek) UPDATE: NM, looks like the tox changes removed testing Twisted < 13.2.0 - I'll remove this try/except block.
The HasHeaders.__repr__ function is uncovered - I'll add a test for that.
test_testing.py is checking for treq.collect when instead it should be treq.content

…treq.content

glyph · 2015-07-08T21:33:25Z

abortConnection's call to loseConnection: I guess abortConnection is never called - I'm not sure how to trigger that in the tests - will that work if we just cancel an outstanding request?

That should be what happens, yes.

…ed the version requirement of twisted up to 13.2.0

…ponseStubs

glyph · 2015-07-08T22:56:08Z

IStringResponseStubs.get_response_for - I'd expect this to not be covered at all, since it's an interface - I'm not sure why it's partially covered. I guess we should add a # pragma no-cover or something to this line?

I think that a pragma is a reasonable choice for now.

…d ensure that it only accepts bytes or unicode.

…add test for AbortableStringTransport.abortConnection

Conflicts: setup.py

cyli · 2015-07-09T00:37:49Z

treq/test/test_testing.py

+                         self.successResultOf(stub.content(resp)))
+
+
+class SequenceStringStubsTests(TestCase):


Note to reviewers: I plan to write docs in a later PR, because don't want to make this PR already bigger than the monster it already is. But the API should look something like how these tests are written - maybe we can provide a convenience API like string_stub_treq([((expected request args), (expected response))]) instead of having to construct StubTreq(StringStubbingResource(SequenceStringStubs([...])))?

…erted in a future PR to add this functionality back in.

cyli · 2015-07-09T00:58:57Z

Note to reviewer - I removed some code to make this PR smaller to review. For reference, 75b49b1 is the code that was removed, and is what I intend to submit for the next PR (should give some idea of what the API for stubbing responses should look like)

hynek · 2015-07-09T05:14:54Z

Two things I wouldn’t know where else to mention:

great to see that codecov already proved useful :)
IMHO we should put all interfaces into a separate file (_interfaces.py?) and exclude the whole file in coverage instead of littering the code with pragmas

Conflicts: requirements-dev.txt setup.py

radix · 2015-07-09T20:56:45Z

treq/test/test_testing.py

+
+    def test_providing_resource_to_stub_treq(self):
+        """
+        The resource provided to StubTreq is responds to every request no


stray "is" here

radix · 2015-07-09T21:21:56Z

Couple of things I noticed while tinkering around in my REPL:

if I request an invalid URL like "" or "abc", I get a synchronous IndexError. If I make a request like that with treq I get a Deferred with a SchemeNotSupported error.
I made a request against a blank Resource() to http://google.com/, and then if I make a request to "abc" or "" again, I get a synchronous AlreadyCalledError, which seems strange and could be indicative of a deeper bug.

@radix

…meNotSupported) Thanks @radix!

cyli · 2015-07-12T22:23:29Z

@radix: thanks for finding that bug! Fixed it so it now bails out early if the response from the real agent already has a (failure) response

radix · 2015-07-13T23:09:52Z

I am a little surprised that making requests with https URLs without PyOpenSSL is giving me an "SSL support unavailable" error.

This LGTM!

glyph · 2015-07-13T23:35:19Z

Let's deal with the HTTPS support issue in a future version - that's a problem with treq itself, not this code :).

Add an in-memory stub version of treq

cyli added 11 commits July 2, 2015 14:09

Import RequestTraversalAgent and dependencies from mimic

43e8a5c

Add a StubTreq class which mimics the treq interface, and tests to en…

ec03fd8

…sure that it has the same interface and will return stubbed responses if given an appropriate resource.

Allow injection of an IBodyProducer callable into the HTTP client, so…

0e87015

… that we can have a synchronous IBodyProducer instead of something that requires a cooperator.

Remove invalid scheme in tests, because that won't even work. HEAD re…

d454d77

…quests should have no body.

Support stubbing HTTPS URLs as well.

a1607d6

Include test that passing the file kwarg raises an AssertionError.

c7aac7a

Add a default resource that accepts string bodies and returns string …

575cf6f

…responses.

Add HasHeaders, because Twisted Agent adds extra request headers, so …

754fc7d

…we want to check if the expected headers is a strict subset of the actual headers. Fix bugs.

Add a type of IStringResponseStub that takes a sequence of expected r…

381febd

…equests and responses.

Fix bug with HTTPS scheme not appearing in the request URL

63dcb1e

Different error/failure reporting method.

b6591b0

cyli mentioned this pull request Jul 7, 2015

Contribute StubTreq and StubResponse to upstream treq as a testing utility rackerlabs/otter#1423

Closed

Merge remote-tracking branch 'origin/master' into fake-treq

ca78637

glyph reviewed Jul 8, 2015
View reviewed changes

Rename bodyproducer to data_to_body_producer as per review comment

dadb419

cyli force-pushed the fake-treq branch from 7e9d15c to dadb419 Compare July 8, 2015 18:29

cyli added 2 commits July 8, 2015 14:19

Fix bug in test that was referring to treq.collect when it should be …

649d27e

…treq.content

Merge branch 'master' into fake-treq

886bd1f

cyli added 4 commits July 8, 2015 15:14

Add tests for HasHeaders case sensitivity and repr-ing

f0fb775

Remove the try/except block around importing IAgent, since we've bump…

c9b1cba

…ed the version requirement of twisted up to 13.2.0

Add test to verify SequenceStringStubs actually implements IStringRes…

2b073c6

…ponseStubs

Fix StubTreq API verification so that it has full branch coverage.

bf63d03

cyli added 3 commits July 8, 2015 17:14

Change SynchronousProducer to be private, since it's inaccessible, an…

de192a4

…d ensure that it only accepts bytes or unicode.

Make RequestTraversalAgent handle asynchronous requests as well, and …

1c11f8f

…add test for AbortableStringTransport.abortConnection

Merge branch 'master' into fake-treq

fca2a9b

Conflicts: setup.py

cyli reviewed Jul 9, 2015
View reviewed changes

Remove some code, to make it easier to review. This commit can be rev…

75b49b1

…erted in a future PR to add this functionality back in.

Merge branch 'master' into fake-treq

cc9196c

Conflicts: requirements-dev.txt setup.py

radix reviewed Jul 9, 2015
View reviewed changes

Handle the case where the response already has a result (such as Sche…

5d50066

…meNotSupported) Thanks @radix!

hynek mentioned this pull request Jul 13, 2015

Pin mock to 1.0.1 #105

Merged

Merge branch 'master' into fake-treq

c672632

glyph added a commit that referenced this pull request Jul 13, 2015

Merge pull request #96 from twisted/fake-treq

6daab31

Add an in-memory stub version of treq

glyph merged commit 6daab31 into master Jul 13, 2015

cyli deleted the fake-treq branch July 13, 2015 23:39

cyli mentioned this pull request Jul 13, 2015

Utility for providing a sequence of requests/responses to StubTreq #106

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding an in-memory stub version of treq #96

Adding an in-memory stub version of treq #96

cyli commented Jul 7, 2015

cyli commented Jul 7, 2015

glyph commented Jul 8, 2015

glyph commented Jul 8, 2015

glyph Jul 8, 2015

codecov-io commented Jul 8, 2015

glyph commented Jul 8, 2015

hynek commented Jul 8, 2015

cyli commented Jul 8, 2015

glyph commented Jul 8, 2015

glyph commented Jul 8, 2015

cyli Jul 9, 2015

cyli commented Jul 9, 2015

hynek commented Jul 9, 2015

radix Jul 9, 2015

radix commented Jul 9, 2015

cyli commented Jul 12, 2015

radix commented Jul 13, 2015

glyph commented Jul 13, 2015

		self.successResultOf(stub.content(resp)))


		class SequenceStringStubsTests(TestCase):

Adding an in-memory stub version of treq #96

Adding an in-memory stub version of treq #96

Conversation

cyli commented Jul 7, 2015

cyli commented Jul 7, 2015

glyph commented Jul 8, 2015

glyph commented Jul 8, 2015

glyph Jul 8, 2015

Choose a reason for hiding this comment

codecov-io commented Jul 8, 2015

Current coverage is 95.82%

Coverage Diff

glyph commented Jul 8, 2015

hynek commented Jul 8, 2015

cyli commented Jul 8, 2015

glyph commented Jul 8, 2015

glyph commented Jul 8, 2015

cyli Jul 9, 2015

Choose a reason for hiding this comment

cyli commented Jul 9, 2015

hynek commented Jul 9, 2015

radix Jul 9, 2015

Choose a reason for hiding this comment

radix commented Jul 9, 2015

cyli commented Jul 12, 2015

radix commented Jul 13, 2015

glyph commented Jul 13, 2015

Current coverage is `95.82%`