Skip to content
This repository has been archived by the owner on May 26, 2021. It is now read-only.

#47 empty headers in admin #48

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

PetrS12
Copy link
Contributor

@PetrS12 PetrS12 commented Oct 18, 2020

closes #47

@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #48 into develop will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #48      +/-   ##
===========================================
+ Coverage    98.31%   98.34%   +0.02%     
===========================================
  Files            5        5              
  Lines          119      121       +2     
  Branches         4        5       +1     
===========================================
+ Hits           117      119       +2     
  Misses           2        2              
Impacted Files Coverage Δ
http_stubs/views.py 100.00% <100.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 867e290...ea7d7d8. Read the comment docs.

@@ -64,3 +64,14 @@ def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
for header_name, header_value in stub.resp_headers.items():
response[header_name] = header_value
return response

def remove_empty_headers(self, headers: Dict) -> Dict:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should strive to use more accurate type annotations:
Dict[str, str] -> Dict[str, str]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. Thanks!

:param client: http client fixture
"""
http_stub_factory(method=HTTPMethod.GET.name)
client.get('/default_path/')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please move this string ('/default_path/') to a separate variable and use wherever it should be used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. Thanks!

http_stub_factory(method=HTTPMethod.GET.name)
client.get('/default_path/')
log = LogEntry.objects.last()
assert log.headers == {} # noqa:WPS520
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert log.headers == {} # noqa:WPS520
assert bool(log.headers) is False

I think, it would be better this way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. Thanks!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

http_stub_factory(method=HTTPMethod.GET.name)
client.get(self.path)
log = LogEntry.objects.last()
assert bool(log.headers) is False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking, but:

Suggested change
assert bool(log.headers) is False
assert not bool(log.headers)

@@ -64,3 +64,14 @@ def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
for header_name, header_value in stub.resp_headers.items():
response[header_name] = header_value
return response

def remove_empty_headers(self, headers: Dict[str, str]) -> Dict[str, str]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just asking: can't be empty headers significant? E.g. indicate there's something wrong on the sending side?

@lowitea lowitea added the bug Something isn't working label Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty headers are displayed in the admin page
3 participants