Conversation
|
|
||
| if not is_payload_collection_blacklisted(full_url): | ||
| if ( | ||
| response.tell() > 0 and |
There was a problem hiding this comment.
I think the headers are still useful even if the response is empty, WDYT?
| :return: None | ||
| """ | ||
|
|
||
| original = epsagon.http_filters.BLACKLIST_URLS # Storing old state |
There was a problem hiding this comment.
Extract to pytest.fixture like:
original = epsagon.http_filters.BLACKLIST_URLS
yield
epsagon.http_filters.BLACKLIST_URLS = original
(instead of making it a part of this specific test)
| } | ||
|
|
||
| http = urllib3.PoolManager() | ||
| return http.request('POST', TEST_URL, headers=headers, body='', preload_content=True) |
There was a problem hiding this comment.
POST? (was expecting a GET)- What happens if preload_content=False?
There was a problem hiding this comment.
- Fixed
- Added another test
| http = urllib3.PoolManager() | ||
| conn = http.connection_from_url(TEST_URL) | ||
| urllib_response = conn.urlopen( | ||
| method='POST', |
There was a problem hiding this comment.
maybe better to test both cause the body may be handled differently
|
|
||
| response = wrapped_function() | ||
| data = response.read() | ||
| # In this case data will not be available in the buffer |
There was a problem hiding this comment.
Is this a valid behavior (does it behave the same way without the Epsagon wrapper)?
I suggest a different test - can we run the same request twice, once with Epsagon wrapper and once without it, and compare the response objects? (same for the conn.urlopen test)
| urllib3_event = trace_transport.last_trace.events[-1] | ||
| # Payload will be collected and stored inside the event data | ||
| assert(len(urllib3_event.resource['metadata']['response_body']) > 0) | ||
|
|
There was a problem hiding this comment.
assert (response.data == wrapped_data)
Should we also check the peek attribute of the responses to make sure they are the same?
There was a problem hiding this comment.
You can't do that since the data is different
There was a problem hiding this comment.
I'm sure we can find a site for testing that returns the same data each time.
Such a test would be very powerful, and would make sure the original response is not harmed by the instrumentation
| assert(len(urllib3_event.resource['metadata']['response_body']) > 0) | ||
|
|
||
|
|
||
| def test_data_capture_with_pool_manager_pc_false(trace_transport): |
There was a problem hiding this comment.
Can be a parameterized test with a preload_content arg:
@pytest.mark.parametrize("preload_content", [true, false])
def test_data_capture_with_pool_manager(preload_content):
Instead of two different tests, no?
(and compare the wrapped and non wrapped .read() as well as .data
|
🎉 This PR is included in version 1.68.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This reverts commit b5a56c3.
4 participants
No description provided.