50581 2010-12-06 12:21:18 -0800 Make TestResultsServer return empty JSON or 404 for non-existing results files 2010-12-06 16:07:22 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 kinuko webkit-unassigned ojan oldest_to_newest 317780 0 kinuko 2010-12-06 12:21:18 -0800 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). 317786 1 75726 kinuko 2010-12-06 12:23:33 -0800 Created attachment 75726 Patch 317793 2 75726 ojan 2010-12-06 12:32:44 -0800 Comment on attachment 75726 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? 317802 3 75726 kinuko 2010-12-06 12:42:58 -0800 Comment on attachment 75726 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? 317805 4 ojan 2010-12-06 12:46:52 -0800 (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/ 317809 5 kinuko 2010-12-06 13:01:33 -0800 (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 317837 6 ojan 2010-12-06 14:05:47 -0800 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. 317845 7 ojan 2010-12-06 14:22:31 -0800 Bug 50589 for the onerror bug. 317882 8 75742 kinuko 2010-12-06 15:38:26 -0800 Created attachment 75742 Patch 317884 9 kinuko 2010-12-06 15:42:54 -0800 (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. 317900 10 kinuko 2010-12-06 16:07:22 -0800 Committed r73409: <http://trac.webkit.org/changeset/73409> 75726 2010-12-06 12:23:33 -0800 2010-12-06 15:38:24 -0800 Patch bug-50581-20101206122332.patch text/plain 1923 kinuko ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCBmOTc3OGU1NWRlNjU1MTI3Y2FiOGQ3MTgyYTcyZmQ2MzFkZDFmYzQ1Li45NTcwMGMw MzgxN2Y2NGQzZDU0MmJhNDNlZDE2MGY4NzlkN2I3Njk2IDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAK KzIwMTAtMTItMDYgIEtpbnVrbyBZYXN1ZGEgIDxraW51a29AY2hyb21pdW0ub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1ha2UgVGVzdFJlc3Vs dHNTZXJ2ZXIgcmV0dXJuIGVtcHR5IEpTT04gb3IgNDA0IGZvciBub24tZXhpc3RpbmcgcmVzdWx0 cyBmaWxlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTUwNTgxCisKKyAgICAgICAgKiBUZXN0UmVzdWx0U2VydmVyL2hhbmRsZXJzL3Rlc3RmaWxlaGFu ZGxlci5weToKKwogMjAxMC0xMi0wMiAgT2phbiBWYWZhaSAgPG9qYW5AY2hyb21pdW0ub3JnPgog CiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9v bHMvVGVzdFJlc3VsdFNlcnZlci9oYW5kbGVycy90ZXN0ZmlsZWhhbmRsZXIucHkgYi9XZWJLaXRU b29scy9UZXN0UmVzdWx0U2VydmVyL2hhbmRsZXJzL3Rlc3RmaWxlaGFuZGxlci5weQppbmRleCAx ZTMxMjA2MTNjYzVjZTZjYWIxZmVjNTRlMzA0YTVmMGViM2E1YmRjLi4xN2FjM2NkNjkwN2RmMGVi YTY5YThjZGM2OTFmZWZkZThkNWJkNDU0IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9UZXN0UmVz dWx0U2VydmVyL2hhbmRsZXJzL3Rlc3RmaWxlaGFuZGxlci5weQorKysgYi9XZWJLaXRUb29scy9U ZXN0UmVzdWx0U2VydmVyL2hhbmRsZXJzL3Rlc3RmaWxlaGFuZGxlci5weQpAQCAtNDYsNiArNDYs OCBAQCBQQVJBTV9URVNUX1RZUEUgPSAidGVzdHR5cGUiCiBQQVJBTV9JTkNSRU1FTlRBTCA9ICJp bmNyZW1lbnRhbCIKIFBBUkFNX1RFU1RfTElTVF9KU09OID0gInRlc3RsaXN0anNvbiIKIAorRU1Q VFlfSlNPTl9GT1JNQVQgPSAnQUREX1JFU1VMVFMoeyIlcyI6e319KTsnCisKIAogY2xhc3MgRGVs ZXRlRmlsZSh3ZWJhcHAuUmVxdWVzdEhhbmRsZXIpOgogICAgICIiIkRlbGV0ZSB0ZXN0IGZpbGUg Zm9yIGEgZ2l2ZW4gYnVpbGRlciBhbmQgbmFtZSBmcm9tIGRhdGFzdG9yZS4iIiIKQEAgLTExMyw3 ICsxMTUsNyBAQCBjbGFzcyBHZXRGaWxlKHdlYmFwcC5SZXF1ZXN0SGFuZGxlcik6CiAgICAgICAg IGlmIG5vdCBmaWxlczoKICAgICAgICAgICAgIGxvZ2dpbmcuaW5mbygiRmlsZSBub3QgZm91bmQs IG1hc3RlciAlcywgYnVpbGRlcjogJXMsIHRlc3RfdHlwZTogJXMsIG5hbWU6ICVzLiIsCiAgICAg ICAgICAgICAgICAgICAgICAgICAgbWFzdGVyLCBidWlsZGVyLCB0ZXN0X3R5cGUsIG5hbWUpCi0g ICAgICAgICAgICByZXR1cm4gTm9uZQorICAgICAgICAgICAgcmV0dXJuIEVNUFRZX0pTT05fRk9S TUFUICUgYnVpbGRlcgogCiAgICAgICAgIHJldHVybiBmaWxlc1swXS5kYXRhCiAKQEAgLTE1OCw3 ICsxNjAsOCBAQCBjbGFzcyBHZXRGaWxlKHdlYmFwcC5SZXF1ZXN0SGFuZGxlcik6CiAgICAgICAg IGlmIGpzb246CiAgICAgICAgICAgICBzZWxmLnJlc3BvbnNlLmhlYWRlcnNbIkNvbnRlbnQtVHlw ZSJdID0gInRleHQvcGxhaW47IGNoYXJzZXQ9dXRmLTgiCiAgICAgICAgICAgICBzZWxmLnJlc3Bv bnNlLm91dC53cml0ZShqc29uKQotCisgICAgICAgIGVsc2U6CisgICAgICAgICAgICBzZWxmLmVy cm9yKDQwNCkKIAogY2xhc3MgVXBsb2FkKHdlYmFwcC5SZXF1ZXN0SGFuZGxlcik6CiAgICAgIiIi VXBsb2FkIHRlc3QgcmVzdWx0cyBmaWxlIHRvIGRhdGFzdG9yZS4iIiIK 75742 2010-12-06 15:38:26 -0800 2010-12-06 15:40:19 -0800 Patch bug-50581-20101206153825.patch text/plain 1278 kinuko ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCBmOTc3OGU1NWRlNjU1MTI3Y2FiOGQ3MTgyYTcyZmQ2MzFkZDFmYzQ1Li42YWNiNzU4 NDMwMWQxODBlZWRiMWE3OTJmYTc0MThjNzJmODdhY2UwIDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAK KzIwMTAtMTItMDYgIEtpbnVrbyBZYXN1ZGEgIDxraW51a29AY2hyb21pdW0ub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1ha2UgVGVzdFJlc3Vs dHNTZXJ2ZXIgcmV0dXJuIDQwNCBmb3Igbm9uLWV4aXN0aW5nIHJlc3VsdHMgZmlsZXMuCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01MDU4MQorCisgICAg ICAgICogVGVzdFJlc3VsdFNlcnZlci9oYW5kbGVycy90ZXN0ZmlsZWhhbmRsZXIucHk6CisKIDIw MTAtMTItMDIgIE9qYW4gVmFmYWkgIDxvamFuQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZp ZXdlZCBieSBFcmljIFNlaWRlbC4KZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1Rlc3RSZXN1bHRT ZXJ2ZXIvaGFuZGxlcnMvdGVzdGZpbGVoYW5kbGVyLnB5IGIvV2ViS2l0VG9vbHMvVGVzdFJlc3Vs dFNlcnZlci9oYW5kbGVycy90ZXN0ZmlsZWhhbmRsZXIucHkKaW5kZXggMWUzMTIwNjEzY2M1Y2U2 Y2FiMWZlYzU0ZTMwNGE1ZjBlYjNhNWJkYy4uNmYwY2E0NDlmMDA1ODIxMmY1M2EyZjYxMjdlZTYz Mjg1ODY2ZmY1MiAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvVGVzdFJlc3VsdFNlcnZlci9oYW5k bGVycy90ZXN0ZmlsZWhhbmRsZXIucHkKKysrIGIvV2ViS2l0VG9vbHMvVGVzdFJlc3VsdFNlcnZl ci9oYW5kbGVycy90ZXN0ZmlsZWhhbmRsZXIucHkKQEAgLTE1OCw3ICsxNTgsOCBAQCBjbGFzcyBH ZXRGaWxlKHdlYmFwcC5SZXF1ZXN0SGFuZGxlcik6CiAgICAgICAgIGlmIGpzb246CiAgICAgICAg ICAgICBzZWxmLnJlc3BvbnNlLmhlYWRlcnNbIkNvbnRlbnQtVHlwZSJdID0gInRleHQvcGxhaW47 IGNoYXJzZXQ9dXRmLTgiCiAgICAgICAgICAgICBzZWxmLnJlc3BvbnNlLm91dC53cml0ZShqc29u KQotCisgICAgICAgIGVsc2U6CisgICAgICAgICAgICBzZWxmLmVycm9yKDQwNCkKIAogY2xhc3Mg VXBsb2FkKHdlYmFwcC5SZXF1ZXN0SGFuZGxlcik6CiAgICAgIiIiVXBsb2FkIHRlc3QgcmVzdWx0 cyBmaWxlIHRvIGRhdGFzdG9yZS4iIiIK