Summary: | generated unexpected_results.html from unexpected_results.json | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||
Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, dpranke, eric, mihaip, tony | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Ojan Vafai
2011-01-19 16:14:09 PST
Created attachment 79513 [details]
Patch
Created attachment 79514 [details]
screenshot
Comment on attachment 79513 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79513&action=review > Tools/ChangeLog:15 > + Unfortunately, we can't just have unexpected_results.html use unexpected_results.json > + because Chrome doesn't allow XHR to local files. This seems lame. (In reply to comment #3) > (From update of attachment 79513 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79513&action=review > > > Tools/ChangeLog:15 > > + Unfortunately, we can't just have unexpected_results.html use unexpected_results.json > > + because Chrome doesn't allow XHR to local files. > > This seems lame. I agree. Blame abarth. He seems to think security is important or something like that. :) (In reply to comment #3) > Tools/ChangeLog:15 > + Unfortunately, we can't just have unexpected_results.html use unexpected_results.json > + because Chrome doesn't allow XHR to local files. One way around this is to use a local HTTP server, the way the rebaseline server does. In fact, is there a reason why we wouldn't want to share the UI code between that and the (unexpected_)results.html display? Should NRWT just launch the server on failures? Entertainingly enough, KSVG/KDOM used a server for displaying the results.html, so that they could have effectively "rebasline" links which would set the new resutlts as the expected results. :) (In reply to comment #5) > (In reply to comment #3) > > Tools/ChangeLog:15 > > + Unfortunately, we can't just have unexpected_results.html use unexpected_results.json > > + because Chrome doesn't allow XHR to local files. > > One way around this is to use a local HTTP server, the way the rebaseline server does. > > In fact, is there a reason why we wouldn't want to share the UI code between that and the (unexpected_)results.html display? Should NRWT just launch the server on failures? It probably makes sense for them to share UI code. I don't like needing to run a local http server though. For one thing, it's just more complicated than is worthwhile for what this is. Inlining doesn't seem like that big of a deal to me. The other, more important thing, is that this page should work on the bots. I guess we could make that work. If you're loading it from a file URL we do something different than an http URL? In either case, I'll remove the review flag for now. I don't have time to finish this in the next week or two. So feel free to take this over if you feel motivated. Comment on attachment 79513 [details]
Patch
remarking for review as per discussion on #webkit
Comment on attachment 79513 [details] Patch abarth points out that our *.json files are JSONP files. We don't need to XHR Them. WE just include them via a script tag afte defining an ADD_RESULTS function: http://en.wikipedia.org/wiki/JSONP The python code here isn't needed. Created attachment 89492 [details]
Patch
Comment on attachment 89492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89492&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results.html:53 > +var test_base_path = 'http://trac.webkit.org/export/76053/trunk/LayoutTests/'; Pretty sure you don't want to pin this at r76053. Comment on attachment 89492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89492&action=review I think this is a great first step. I look forward to further iteration. I think you should fix the hard-coded layout test results path though. :) > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results.html:53 > +var test_base_path = 'http://trac.webkit.org/export/76053/trunk/LayoutTests/'; There is a symlink for you in the results directory iirc. There is an option in old-run-webkit-tests to generate a results.html with remote links. Committed r83799: <http://trac.webkit.org/changeset/83799> Committed r83806: <http://trac.webkit.org/changeset/83806> FYI ... this patch broke the python unit tests (test-webkitpy). The results.html file doesn't exist if the filesystem object you're using is a mock filesystem. I added a check for existence, but it's unclear what the right thing to do is if the file doesn't exist. Fortunately, this shouldn't happen in practice. |