Closed
Bug 768474
Opened 12 years ago
Closed 12 years ago
Reftest analyser broken
Categories
(Tree Management Graveyard :: TBPL, defect)
Tree Management Graveyard
TBPL
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(4 files)
3.31 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
2.74 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
1.39 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
1.20 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
Constant "Fetching reftest log failed." when trying to use the reftest analyser. Can only think this is due to bug 762120; but yet it worked when I tested using Vagrant booo :-( eg: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=de70e79ced32 Reftest analyser link unescaped: data:text/html,<!DOCTYPE html><title>Loading reftest analyzer for Mozilla-Inbound Rev3 Fedora 12x64 mozilla-inbound opt test reftest</title><link rel="stylesheet" href="https://tbpl.mozilla.org/css/style.css"><body><p class="loading">Retrieving reftest log...</p><script src="https://tbpl.mozilla.org/js/jquery.js"></script><script src="https://tbpl.mozilla.org/js/NetUtils.js"></script><script> function encode(s) { return escape(s).replace(/=/g, "%3d") } NetUtils.loadText("php\/getLogExcerpt.php?id=12998274&type=reftest", function(log) { window.location.replace("https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#log=" + encode(encode(log))) }, function() { $("p").removeClass("loading").text("Fetching reftest log failed.") }, function() { $("p").removeClass("loading").text("Fetching reftest log timed out.") }); </script>
Assignee | ||
Comment 1•12 years ago
|
||
Ah, I know why this worked when testing in Vagrant; I was testing side-by-side with the local filesystem version and hadn't popped the mq patch that I use to set baseURL. Ok, so: The patch in bug 762120 did: - 'NetUtils.loadText(' + self._makeJSString(baseURL + result.reftestLogURL) + ',\n' + + 'NetUtils.loadText(' + self._makeJSString(result.reftestLogURL) + ',\n' + I had read baseURL as Config.baseURL, which would have meant it was an empty string on prod/vagrant so they wouldn't have been affected by this patch. However unhelpfully, baseURL is actually used for the absolute base URL in UserInterface.js (which is different from Config.js's usage of it): var baseURL = Config.baseURL || document.baseURI.replace(/\/[^\/]+$/, '/');
Assignee | ||
Comment 2•12 years ago
|
||
In one of the later patches, I'd like to use 'absoluteBaseURL' for the absolute equivalent of baseURL. The current usage is more about ensuring URLs posted in bug comments are publicly accessible/point to a production instance, so this renames them as such. Happy to adjust 'prod' to 'public' or anything else you can think of that would be better.
Assignee | ||
Updated•12 years ago
|
Attachment #637144 -
Attachment description: Part 1 → A - Rename absolute*URL to prod*URL
Assignee | ||
Comment 3•12 years ago
|
||
We're going to need to calculate the absolute URL in both UserInterface.js & BuildbotDBUser.js, so it makes sense to break it out into Config.js with the other URLs. Since it is a calculated value, I've added it to the bottom of Config.js alongside the build/test name generated variables.
Attachment #637149 -
Flags: review?(mstange)
Assignee | ||
Comment 4•12 years ago
|
||
Makes reftestLogURL use the new calculated absoluteBaseURL, so as to make it work both from the local filesystem & when the backend is hosted locally. Fixes the regression from bug 762120.
Attachment #637150 -
Flags: review?(mstange)
Assignee | ||
Comment 5•12 years ago
|
||
Adds a small note to baseURL, since now we have absoluteBaseURL and prodBaseURL a bit of extra clarification is probably needed. Happy to adjust wording if you can think of something better :-) -- Have tested these four patches using Vagrant (this time popping the baseURL mq, so testing properly) and also from the local filesystem using baseURL.
Attachment #637152 -
Flags: review?(mstange)
Updated•12 years ago
|
Attachment #637144 -
Flags: review?(mstange) → review+
Updated•12 years ago
|
Attachment #637149 -
Flags: review?(mstange) → review+
Updated•12 years ago
|
Attachment #637150 -
Flags: review?(mstange) → review+
Updated•12 years ago
|
Attachment #637152 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Thank you for the fast reviews :-) http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/e7d59dd61842 http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/4e129fd12d92 http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/ad33c32b407c http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/657b73898727
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Product: Webtools → Tree Management
Updated•9 years ago
|
Product: Tree Management → Tree Management Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•