Skip to content

Reformat Server-Timing headers to updated W3C spec. #1211

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

Merged
merged 1 commit into from
Dec 20, 2019
Merged

Reformat Server-Timing headers to updated W3C spec. #1211

merged 1 commit into from
Dec 20, 2019

Conversation

tim-schilling
Copy link
Member

Closes #1209

@jdufresne
Copy link
Contributor

Thanks for the fix. Are you up for adding a test?

@tim-schilling
Copy link
Member Author

Ah, I didn't realize that entire function has no test. I'll add one for that.

@@ -92,6 +93,7 @@ class DebugToolbarIntegrationTestCase(TestCase):
def test_middleware(self):
response = self.client.get("/execute_sql/")
self.assertEqual(response.status_code, 200)
self.assertContains(response, "djDebug")
Copy link
Member Author

Choose a reason for hiding this comment

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

This caused the test to fail until the views returned a response containing </body>.

r'CachePanel_total_time;dur=0;desc="Cache 0 Calls"',
]
for expected in expected_partials:
re.compile(expected).search(server_timing)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing the assert.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cripes. Thanks for catching that.

Fixed basic integration tests. When there was no </body> tag
the toolbar's middleware exited earlier and was never inserted.
@tim-schilling
Copy link
Member Author

Rebased on master.

@codecov
Copy link

codecov bot commented Dec 20, 2019

Codecov Report

Merging #1211 into master will increase coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1211      +/-   ##
==========================================
+ Coverage    86.2%   86.41%   +0.21%     
==========================================
  Files          25       25              
  Lines        1428     1428              
  Branches      204      204              
==========================================
+ Hits         1231     1234       +3     
+ Misses        143      141       -2     
+ Partials       54       53       -1
Impacted Files Coverage Δ
debug_toolbar/middleware.py 90.47% <ø> (ø) ⬆️
debug_toolbar/toolbar.py 91.46% <0%> (+3.65%) ⬆️

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 fd50ce3...3e8cdda. Read the comment docs.

@matthiask matthiask merged commit 9ddf49d into django-commons:master Dec 20, 2019
@tim-schilling tim-schilling deleted the reformat-server-timing-headers branch December 20, 2019 20:23
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.

Server-Timing header formatted incorrectly?
3 participants