treeclosure hook fetches 2MB of HTML to get at a few hundred bytes that are in TreeName/status.html

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
--
enhancement
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: philor, Assigned: philor)

Tracking

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 508071 [details] [diff] [review]
Fix v.1

Even though the treeclosure hook doesn't hit nearly as often as tbpl, so it didn't feel the need to make a separate status.html static file like tbpl did, the hook really should take advantage of it - fetching the whole waterfall to get a the word "CLOSED" is rather excessive.

Untested, since I don't know how to test it, but I did at least verify that all the URLs work.
Attachment #508071 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

6 years ago
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Comment on attachment 508071 [details] [diff] [review]
Fix v.1

Seems totally reasonable, and we already have tests that cover this (they obviously don't hit the live Tinderbox page, or they wouldn't make very good tests), but you will have to adjust the URLs in the tests to match your changes here:
http://hg.mozilla.org/hgcustom/hghooks/file/04b0488534f5/runtests.py#l230

The redirect method there gives a URL and its contents, and causes the test to expect that that URL will be the next one fetched, and return that string as the URL contents:
http://hg.mozilla.org/hgcustom/hghooks/file/04b0488534f5/runtests.py#l192

You can run the tests with "python runtests.py".

r=me with the tests fixed.
Attachment #508071 - Flags: review?(ted.mielczarek) → review+
Created attachment 508154 [details] [diff] [review]
patch for c-c hook as well.
Attachment #508154 - Flags: review?(ted.mielczarek)
Attachment #508154 - Flags: feedback?(philringnalda)
Created attachment 508155 [details] [diff] [review]
[checked in] patch for c-c hook as well. (with test fixed for CC hook)
Attachment #508154 - Attachment is obsolete: true
Attachment #508155 - Flags: review?(ted.mielczarek)
Attachment #508155 - Flags: feedback?(philringnalda)
Attachment #508154 - Flags: review?(ted.mielczarek)
Attachment #508154 - Flags: feedback?(philringnalda)
Attachment #508155 - Flags: review?(ted.mielczarek) → review?(bugzilla)
Attachment #508155 - Flags: review?(bugzilla)
Attachment #508155 - Flags: review+
Attachment #508155 - Flags: feedback?(philringnalda)
Comment on attachment 508155 [details] [diff] [review]
[checked in] patch for c-c hook as well. (with test fixed for CC hook)

I've pushed this as I'm reviewing another patch that is going to add to the tests, so may as well get this url changed first.
Attachment #508155 - Attachment description: patch for c-c hook as well. (with test fixed for CC hook) → [checked in] patch for c-c hook as well. (with test fixed for CC hook)
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/hgcustom/hghooks/rev/63a618e9a3d3
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Depends on: 631957
Product: mozilla.org → Release Engineering

Comment 6

3 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/1e4384187eb4
https://hg.mozilla.org/hgcustom/version-control-tools/rev/eeefa6de5fbb
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.