Make TestResultsServer return empty JSON or 404 for non-existing results files. For non-layout tests some builders do not have complete test results but only for a specific set of tests (e.g. only ui_tests and net_unittests for "XP Tests (dbg)(2)" builder etc). We should return empty json or 404 for such cases (otherwise the dashboard frontend won't run as intended).
Created attachment 75726 [details] Patch
Comment on attachment 75726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75726&action=review > WebKitTools/TestResultServer/handlers/testfilehandler.py:118 > + return EMPTY_JSON_FORMAT % builder Why do we want to do this instead of handling None at the calling location? Don't we want to 404 in this case as well?
Comment on attachment 75726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75726&action=review >> WebKitTools/TestResultServer/handlers/testfilehandler.py:118 >> + return EMPTY_JSON_FORMAT % builder > > Why do we want to do this instead of handling None at the calling location? Don't we want to 404 in this case as well? I want to do so as well, simply returning 404 somehow didn't get the script.onerror function called in my local env (unlike normal 404 error). I haven't investigated it yet -- do you know how to make it work?
(In reply to comment #3) > I want to do so as well, simply returning 404 somehow didn't get the script.onerror function called in my local env (unlike normal 404 error). I haven't investigated it yet -- do you know how to make it work? Hm...should work. Maybe it's some issue with serving off localhost. Maybe try updating the testing server and see if it works there? http://test-results-test.appspot.com/
(In reply to comment #4) > (In reply to comment #3) > > I want to do so as well, simply returning 404 somehow didn't get the script.onerror function called in my local env (unlike normal 404 error). I haven't investigated it yet -- do you know how to make it work? > > Hm...should work. Maybe it's some issue with serving off localhost. Maybe try updating the testing server and see if it works there? > > http://test-results-test.appspot.com/ I updated the test-results-test server but getting the same result. (The console shows 'Failed to load resource: the server responded with a status of 404 (Not Found)' error messages as well as for normal 404 but the error function isn't being hooked up.) Will you be able to take a look at it? On the testing server now URLs like following returns 404. http://test-results-test.appspot.com/testfile?builder=Linux%20Builder%20(Views%20dbg)&master=Chromium&testtype=unit_tests&name=results.json
Looks like a webkit bug. At first glance, it seems we don't fire the error event for a 404 if the file contents are empty. Firefox fires the error event in this case. As a workaround, on the JS side, we can listen to the load event as well and check if the script is empty, e.g., <script id="foo" src="http://test-results-test.appspot.com/testfile?builder=Linux%20Builder%20(Views%20dbg)&master=Chromium&testtype=unit_tests&name=results.json" onload="alert('innerHTML=\'' + document.getElementById('foo').innerHTML + '\'');" onerror="alert('error')"></script> Eventually, we can fix the webkit bug and won't need this anymore.
Bug 50589 for the onerror bug.
Created attachment 75742 [details] Patch
(In reply to comment #6) > Looks like a webkit bug. At first glance, it seems we don't fire the error event for a 404 if the file contents are empty. Firefox fires the error event in this case. As a workaround, on the JS side, we can listen to the load event as well and check if the script is empty, e.g., > > <script > id="foo" > src="http://test-results-test.appspot.com/testfile?builder=Linux%20Builder%20(Views%20dbg)&master=Chromium&testtype=unit_tests&name=results.json" > onload="alert('innerHTML=\'' + document.getElementById('foo').innerHTML + '\'');" > onerror="alert('error')"></script> > > Eventually, we can fix the webkit bug and won't need this anymore. Interesting. onload work-around seemed to work. I updated the patch just to return 404.
Committed r73409: <http://trac.webkit.org/changeset/73409>