Bug 52763 – generated unexpected_results.html from unexpected_results.json

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

generated unexpected_results.html from unexpected_results.json

[Ojan Vafai]

Created attachment 79513 [details]
Patch

[Ojan Vafai]

Created attachment 79514 [details]
screenshot

[Eric Seidel (no email)]

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.

[Ojan Vafai]

(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. :)

[Mihai Parparita]

(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?

[Eric Seidel (no email)]

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. :)

[Ojan Vafai]

(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.

[Ojan Vafai]

Comment on attachment 79513 [details]
Patch

remarking for review as per discussion on #webkit

[Eric Seidel (no email)]

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.

[Ojan Vafai]

Created attachment 89492 [details]
Patch

[Mihai Parparita]

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.

[Eric Seidel (no email)]

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.

[Ojan Vafai]

Committed r83799: <http://trac.webkit.org/changeset/83799>

[Dirk Pranke]

Committed r83806: <http://trac.webkit.org/changeset/83806>

[Dirk Pranke]

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.