71067 2011-10-27 15:00:55 -0700 Test expectation errors should include the file name and platform in both the log and exception info. 2011-10-27 17:29:59 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 levin levin abarth dpranke eric ojan webkit.review.bot oldest_to_newest 491991 0 levin 2011-10-27 15:00:55 -0700 Test expectation errors should include the file name and platform in both the log and exception info. 491993 1 112764 levin 2011-10-27 15:02:54 -0700 Created attachment 112764 Patch 491998 2 levin 2011-10-27 15:06:55 -0700 Added smfr as fyi. A follow up on https://bugs.webkit.org/show_bug.cgi?id=71033#c10 Everyone else is a possible reviewer :) 492006 3 dpranke 2011-10-27 15:16:13 -0700 View in context: https://bugs.webkit.org/attachment.cgi?id=112764&action=review I'm curious as to what the motivation for this patch is? Since it isn't distinguishing between regular expectations and overrides, I'm guessing you're doing something where you want to see which port's expectations file you're using? In the normal run-webkit-tests case, is this necessary? Or are you doing something like linting the test files where you're likely to encounter multiple expectations files? > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:790 > + failure_title = "FAILURES FOR %s in %s" % (str(self._test_config), test_expectation_relative_path) You can use test_expectation_relative_path = self._port.relative_test_filename(self._port.path_to_test_expectations_file()) as long as the expectations file is under LayoutTests (which AFAIK they all are). That won't include the "LayoutTests" part, which may or may not be desirable. 492016 4 levin 2011-10-27 15:28:32 -0700 (In reply to comment #3) > View in context: https://bugs.webkit.org/attachment.cgi?id=112764&action=review > > I'm curious as to what the motivation for this patch is? Since it isn't distinguishing between regular expectations and overrides, I'm guessing you're doing something where you want to see which port's expectations file you're using? In the normal run-webkit-tests case, is this necessary? Or are you doing something like linting the test files where you're likely to encounter multiple expectations files? Linting the test files. For example, this happens during a unit test run right now on OS X. Now this is a normal OS X (not Chromium OS X) bot, so when we got a failure, I tried to look at the test_expectations for platform/mac but then found that it was complaining about the chromium one. Also in general if a test case is failing due to this, it would be nice to know which test_expectations is causing it (since we have several of them checked in). > > > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:790 > > + failure_title = "FAILURES FOR %s in %s" % (str(self._test_config), test_expectation_relative_path) > > You can use test_expectation_relative_path = self._port.relative_test_filename(self._port.path_to_test_expectations_file()) as long as the expectations file is under LayoutTests (which AFAIK they all are). That won't include the "LayoutTests" part, which may or may not be desirable. I like the path from the root as it feels less "magical". I see LayoutTests/ and I know where that is. If I see "platform/", I'm not immediately sure since there are multiple platform dirs. 492022 5 dpranke 2011-10-27 15:35:27 -0700 (In reply to comment #4) > (In reply to comment #3) > > View in context: https://bugs.webkit.org/attachment.cgi?id=112764&action=review > > > > I'm curious as to what the motivation for this patch is? Since it isn't distinguishing between regular expectations and overrides, I'm guessing you're doing something where you want to see which port's expectations file you're using? In the normal run-webkit-tests case, is this necessary? Or are you doing something like linting the test files where you're likely to encounter multiple expectations files? > > Linting the test files. > > For example, this happens during a unit test run right now on OS X. Now this is a normal OS X (not Chromium OS X) bot, so when we got a failure, I tried to look at the test_expectations for platform/mac but then found that it was complaining about the chromium one. > > Also in general if a test case is failing due to this, it would be nice to know which test_expectations is causing it (since we have several of them checked in). > Perhaps. I'm on the fence as to whether this will be unnecessary noise or not in the common case, but adding one line is not a big deal and it's clearly useful in the linting-the-files case. > > > > > > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:790 > > > + failure_title = "FAILURES FOR %s in %s" % (str(self._test_config), test_expectation_relative_path) > > > > You can use test_expectation_relative_path = self._port.relative_test_filename(self._port.path_to_test_expectations_file()) as long as the expectations file is under LayoutTests (which AFAIK they all are). That won't include the "LayoutTests" part, which may or may not be desirable. > > I like the path from the root as it feels less "magical". I see LayoutTests/ and I know where that is. If I see "platform/", I'm not immediately sure since there are multiple platform dirs. Sure, I figured you'd say something like that :) That's fine, just thought I'd mention the alternative. 492024 6 levin 2011-10-27 15:38:08 -0700 (In reply to comment #5) > Perhaps. I'm on the fence as to whether this will be unnecessary noise or not in the common case, but adding one line is not a big deal and it's clearly useful in the linting-the-files case. Yeah, checkout https://bugs.webkit.org/show_bug.cgi?id=71033#c0 I really dislike tests that fail and leave you guessing about how to fix them so this should help. 492126 7 112764 webkit.review.bot 2011-10-27 17:29:54 -0700 Comment on attachment 112764 Patch Clearing flags on attachment: 112764 Committed r98666: <http://trac.webkit.org/changeset/98666> 492127 8 webkit.review.bot 2011-10-27 17:29:59 -0700 All reviewed patches have been landed. Closing bug. 112764 2011-10-27 15:02:54 -0700 2011-10-27 17:29:54 -0700 Patch bug-71067-20111027150252.patch text/plain 4771 levin U3VidmVyc2lvbiBSZXZpc2lvbjogOTg2MDIKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCA2YTY1OWFmYzRkNzk4MGMzODkwYmQ4OWRkZTk2M2M5MGFl MTY2Zjc2Li5hZWIxMTk4MTMyMzliNDk0OTEwYWRhODhhNThhMWJjYWI1NTRjNzRmIDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTEtMTAtMjcgIERhdmlkIExldmluICA8bGV2aW5AY2hyb21pdW0ub3JnPgorCisgICAg ICAgIFRlc3QgZXhwZWN0YXRpb24gZXJyb3JzIHNob3VsZCBpbmNsdWRlIHRoZSBmaWxlIG5hbWUg YW5kIHBsYXRmb3JtIGluIGJvdGggdGhlIGxvZyBhbmQgZXhjZXB0aW9uIGluZm8uCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MTA2NworCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogU2NyaXB0cy93ZWJraXRw eS9sYXlvdXRfdGVzdHMvbW9kZWxzL3Rlc3RfZXhwZWN0YXRpb25zLnB5OgorICAgICAgICAqIFNj cmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL21vZGVscy90ZXN0X2V4cGVjdGF0aW9uc191bml0 dGVzdC5weToKKwogMjAxMS0xMC0yNyAgQWRhbSBSb2JlbiAgPGFyb2JlbkBhcHBsZS5jb20+CiAK ICAgICAgICAgVGVzdCBXS0J1bmRsZVBhZ2VHZXRCYWNraW5nU2NhbGVGYWN0b3IKZGlmZiAtLWdp dCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL21vZGVscy90ZXN0X2V4cGVj dGF0aW9ucy5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL21vZGVscy90 ZXN0X2V4cGVjdGF0aW9ucy5weQppbmRleCA3NWJmNjg4OTJhNjYxYTVhODcxZGE0MTc3ZDEyNjlm MzcwMzA3MDliLi41M2IzZDc3ZTQwOTg1YTY1Mzg3YTc4OTMxZjg3NjNhYmMyYmE3NTJjIDEwMDY0 NAotLS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9tb2RlbHMvdGVzdF9l eHBlY3RhdGlvbnMucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMv bW9kZWxzL3Rlc3RfZXhwZWN0YXRpb25zLnB5CkBAIC02OTUsNiArNjk1LDcgQEAgY2xhc3MgVGVz dEV4cGVjdGF0aW9ucyhvYmplY3QpOgogICAgICAgICBzZWxmLl9pc19saW50X21vZGUgPSBpc19s aW50X21vZGUKICAgICAgICAgc2VsZi5fbW9kZWwgPSBUZXN0RXhwZWN0YXRpb25zTW9kZWwoKQog ICAgICAgICBzZWxmLl9wYXJzZXIgPSBUZXN0RXhwZWN0YXRpb25QYXJzZXIocG9ydCwgdGVzdHMs IGlzX2xpbnRfbW9kZSkKKyAgICAgICAgc2VsZi5fcG9ydCA9IHBvcnQKICAgICAgICAgc2VsZi5f dGVzdF9jb25maWd1cmF0aW9uX2NvbnZlcnRlciA9IFRlc3RDb25maWd1cmF0aW9uQ29udmVydGVy KHBvcnQuYWxsX3Rlc3RfY29uZmlndXJhdGlvbnMoKSwgcG9ydC5jb25maWd1cmF0aW9uX3NwZWNp Zmllcl9tYWNyb3MoKSkKIAogICAgICAgICBzZWxmLl9leHBlY3RhdGlvbnMgPSBUZXN0RXhwZWN0 YXRpb25QYXJzZXIudG9rZW5pemVfbGlzdChleHBlY3RhdGlvbnMpCkBAIC03ODMsNyArNzg0LDEx IEBAIGNsYXNzIFRlc3RFeHBlY3RhdGlvbnMob2JqZWN0KToKICAgICAgICAgICAgICAgICB3YXJu aW5ncy5hcHBlbmQoIkxpbmU6JXMgJXMgJXMiICUgKGV4cGVjdGF0aW9uLmxpbmVfbnVtYmVyLCB3 YXJuaW5nLCBleHBlY3RhdGlvbi5uYW1lIGlmIGV4cGVjdGF0aW9uLmV4cGVjdGF0aW9ucyBlbHNl IGV4cGVjdGF0aW9uLm9yaWdpbmFsX3N0cmluZykpCiAKICAgICAgICAgaWYgbGVuKGVycm9ycykg b3IgbGVuKHdhcm5pbmdzKToKLSAgICAgICAgICAgIF9sb2cuZXJyb3IoIkZBSUxVUkVTIEZPUiAl cyIgJSBzdHIoc2VsZi5fdGVzdF9jb25maWcpKQorICAgICAgICAgICAgd2Via2l0X2Jhc2VfcGF0 aCA9IHNlbGYuX3BvcnQud2Via2l0X2Jhc2UoKQorICAgICAgICAgICAgdGVzdF9leHBlY3RhdGlv bl9wYXRoID0gc2VsZi5fcG9ydC5wYXRoX3RvX3Rlc3RfZXhwZWN0YXRpb25zX2ZpbGUoKQorICAg ICAgICAgICAgdGVzdF9leHBlY3RhdGlvbl9yZWxhdGl2ZV9wYXRoID0gdGVzdF9leHBlY3RhdGlv bl9wYXRoW2xlbih3ZWJraXRfYmFzZV9wYXRoKTpdLmxzdHJpcCgnLycpCisgICAgICAgICAgICBm YWlsdXJlX3RpdGxlID0gIkZBSUxVUkVTIEZPUiAlcyBpbiAlcyIgJSAoc3RyKHNlbGYuX3Rlc3Rf Y29uZmlnKSwgdGVzdF9leHBlY3RhdGlvbl9yZWxhdGl2ZV9wYXRoKQorICAgICAgICAgICAgX2xv Zy5lcnJvcihmYWlsdXJlX3RpdGxlKQogCiAgICAgICAgICAgICBmb3IgZXJyb3IgaW4gZXJyb3Jz OgogICAgICAgICAgICAgICAgIF9sb2cuZXJyb3IoZXJyb3IpCkBAIC03OTEsMTEgKzc5NiwxMSBA QCBjbGFzcyBUZXN0RXhwZWN0YXRpb25zKG9iamVjdCk6CiAgICAgICAgICAgICAgICAgX2xvZy5l cnJvcih3YXJuaW5nKQogCiAgICAgICAgICAgICBpZiBsZW4oZXJyb3JzKToKLSAgICAgICAgICAg ICAgICByYWlzZSBQYXJzZUVycm9yKGZhdGFsPVRydWUsIGVycm9ycz1lcnJvcnMpCisgICAgICAg ICAgICAgICAgcmFpc2UgUGFyc2VFcnJvcihmYXRhbD1UcnVlLCBlcnJvcnM9W2ZhaWx1cmVfdGl0 bGVdICsgZXJyb3JzKQogICAgICAgICAgICAgaWYgbGVuKHdhcm5pbmdzKToKICAgICAgICAgICAg ICAgICBzZWxmLl9oYXNfd2FybmluZ3MgPSBUcnVlCiAgICAgICAgICAgICAgICAgaWYgc2VsZi5f aXNfbGludF9tb2RlOgotICAgICAgICAgICAgICAgICAgICByYWlzZSBQYXJzZUVycm9yKGZhdGFs PUZhbHNlLCBlcnJvcnM9d2FybmluZ3MpCisgICAgICAgICAgICAgICAgICAgIHJhaXNlIFBhcnNl RXJyb3IoZmF0YWw9RmFsc2UsIGVycm9ycz1bZmFpbHVyZV90aXRsZV0gKyB3YXJuaW5ncykKIAog ICAgIGRlZiBfcHJvY2Vzc190ZXN0c193aXRob3V0X2V4cGVjdGF0aW9ucyhzZWxmKToKICAgICAg ICAgaWYgc2VsZi5fZnVsbF90ZXN0X2xpc3Q6CmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L2xheW91dF90ZXN0cy9tb2RlbHMvdGVzdF9leHBlY3RhdGlvbnNfdW5pdHRlc3QucHkg Yi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9tb2RlbHMvdGVzdF9leHBlY3Rh dGlvbnNfdW5pdHRlc3QucHkKaW5kZXggYjdmODY1ZWYwM2ZjNzM1OWI3NzBmNjBhMGJjYTI2MGU5 ZmEzNmY1ZC4uY2NmNGJhNTNiMGQwMjkwNzMyMjUwMzM5YjBiOGRjYjliNTE5NjQxNiAxMDA2NDQK LS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvbW9kZWxzL3Rlc3RfZXhw ZWN0YXRpb25zX3VuaXR0ZXN0LnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0 X3Rlc3RzL21vZGVscy90ZXN0X2V4cGVjdGF0aW9uc191bml0dGVzdC5weQpAQCAtMTk5LDcgKzE5 OSw4IEBAIFNLSVAgOiBmYWlsdXJlcy9leHBlY3RlZC9pbWFnZS5odG1sIiIiKQogICAgICAgICAg ICAgc2VsZi5hc3NlcnRGYWxzZShUcnVlLCAiUGFyc2VFcnJvciB3YXNuJ3QgcmFpc2VkIikKICAg ICAgICAgZXhjZXB0IFBhcnNlRXJyb3IsIGU6CiAgICAgICAgICAgICBzZWxmLmFzc2VydFRydWUo ZS5mYXRhbCkKLSAgICAgICAgICAgIGV4cF9lcnJvcnMgPSBbdSJMaW5lOjEgVW5yZWNvZ25pemVk IG1vZGlmaWVyICdmb28nIGZhaWx1cmVzL2V4cGVjdGVkL3RleHQuaHRtbCIsCisgICAgICAgICAg ICBleHBfZXJyb3JzID0gW3UiRkFJTFVSRVMgRk9SICVzIGluIExheW91dFRlc3RzL3BsYXRmb3Jt L3Rlc3QvdGVzdF9leHBlY3RhdGlvbnMudHh0IiAlIHNlbGYuX3BvcnQudGVzdF9jb25maWd1cmF0 aW9uKCksCisgICAgICAgICAgICAgICAgICAgICAgICAgIHUiTGluZToxIFVucmVjb2duaXplZCBt b2RpZmllciAnZm9vJyBmYWlsdXJlcy9leHBlY3RlZC90ZXh0Lmh0bWwiLAogICAgICAgICAgICAg ICAgICAgICAgICAgICB1IkxpbmU6MiBNaXNzaW5nIGV4cGVjdGF0aW9ucyBTS0lQIDogZmFpbHVy ZXMvZXhwZWN0ZWQvaW1hZ2UuaHRtbCJdCiAgICAgICAgICAgICBzZWxmLmFzc2VydEVxdWFsKHN0 cihlKSwgJ1xuJy5qb2luKG1hcChzdHIsIGV4cF9lcnJvcnMpKSkKICAgICAgICAgICAgIHNlbGYu YXNzZXJ0RXF1YWwoZS5lcnJvcnMsIGV4cF9lcnJvcnMpCkBAIC0yMTEsNyArMjEyLDggQEAgU0tJ UCA6IGZhaWx1cmVzL2V4cGVjdGVkL2ltYWdlLmh0bWwiIiIpCiAgICAgICAgICAgICBzZWxmLmFz c2VydEZhbHNlKFRydWUsICJQYXJzZUVycm9yIHdhc24ndCByYWlzZWQiKQogICAgICAgICBleGNl cHQgUGFyc2VFcnJvciwgZToKICAgICAgICAgICAgIHNlbGYuYXNzZXJ0RmFsc2UoZS5mYXRhbCkK LSAgICAgICAgICAgIGV4cF9lcnJvcnMgPSBbdSdMaW5lOjEgVGVzdCBsYWNrcyBCVUcgbW9kaWZp ZXIuIGZhaWx1cmVzL2V4cGVjdGVkL3RleHQuaHRtbCddCisgICAgICAgICAgICBleHBfZXJyb3Jz ID0gW3UnRkFJTFVSRVMgRk9SICVzIGluIExheW91dFRlc3RzL3BsYXRmb3JtL3Rlc3QvdGVzdF9l eHBlY3RhdGlvbnMudHh0JyAlIHNlbGYuX3BvcnQudGVzdF9jb25maWd1cmF0aW9uKCksCisgICAg ICAgICAgICAgICAgICAgICAgICAgIHUnTGluZToxIFRlc3QgbGFja3MgQlVHIG1vZGlmaWVyLiBm YWlsdXJlcy9leHBlY3RlZC90ZXh0Lmh0bWwnXQogICAgICAgICAgICAgc2VsZi5hc3NlcnRFcXVh bChzdHIoZSksICdcbicuam9pbihtYXAoc3RyLCBleHBfZXJyb3JzKSkpCiAgICAgICAgICAgICBz ZWxmLmFzc2VydEVxdWFsKGUuZXJyb3JzLCBleHBfZXJyb3JzKQogCg==