Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logging of passthrus when there's no match #505

Merged
merged 2 commits into from Mar 4, 2022
Merged

Add logging of passthrus when there's no match #505

merged 2 commits into from Mar 4, 2022

Conversation

aburgel
Copy link
Contributor

@aburgel aburgel commented Feb 21, 2022

A couple of changes related to passthrus:

responses.passthru_prefixes is always empty because it is a value type that is captured when the responses module is loaded. I've fixed this by making passthru_prefixes a mutable list. Of course, this will allow users to add things directly to the list. The downsides of this seem minimal, but I'm happy to fix this in another way if we think it's important to maintain immutability.

I also added some logging of passthrus when there is no match. I found this helpful for debugging.

@beliaev-maksim
Copy link
Collaborator

I am -1 on making prefixes mutable.
We already had issue with leakage of prefixes

this issue is more related to the PR #497. I would add responses.passthru_prefixes to deprecation

please use responses.mock.passthru_prefixes instead

@aburgel aburgel changed the title Fix exported passthru_prefixes and log passthrus when there's no match Add logging of passthrus when there's no match Feb 21, 2022
@aburgel
Copy link
Contributor Author

aburgel commented Feb 21, 2022

I am -1 on making prefixes mutable.

I removed those changes. Now this PR just adds logging of passthrus when there's no match.

@beliaev-maksim
Copy link
Collaborator

beliaev-maksim commented Feb 22, 2022

@aburgel
Did you follow contribution guide? You can easily set dev environment following it.

Then, please, run tox and fix test coverage by covering added lines

@aburgel
Copy link
Contributor Author

aburgel commented Feb 22, 2022

@beliaev-maksim I updated a test to get back to 100% coverage

@markstory markstory self-assigned this Feb 22, 2022
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #505 (b3d06f9) into master (f039748) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #505   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines         2162      2206   +44     
=========================================
+ Hits          2162      2206   +44     
Impacted Files Coverage Δ
responses/__init__.py 100.00% <100.00%> (ø)
responses/test_responses.py 100.00% <100.00%> (ø)
responses/matchers.py 100.00% <0.00%> (ø)
responses/test_matchers.py 100.00% <0.00%> (ø)

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 f039748...b3d06f9. Read the comment docs.

@markstory markstory merged commit b26bd03 into getsentry:master Mar 4, 2022
markstory added a commit that referenced this pull request Mar 4, 2022
@aburgel aburgel deleted the aburgel/passthru branch March 4, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants