90400 2012-07-02 13:51:57 -0700 Make the skia_test_expectations.txt file optional. 2012-07-03 10:22:02 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ojan ojan abarth dpranke eae epoger eric tony webkit.review.bot oldest_to_newest 661698 0 ojan 2012-07-02 13:51:57 -0700 Make the skia_test_expectations.txt file optional. 661701 1 150471 ojan 2012-07-02 13:53:42 -0700 Created attachment 150471 Patch 661721 2 eae 2012-07-02 14:27:03 -0700 This is great, thank you! 661726 3 150471 dpranke 2012-07-02 14:36:04 -0700 Comment on attachment 150471 Patch So, I'm conflicted about this change ... r-/cq-'ing it for a chance at discussion. The skia test_expectations file was never optional. The chromium downstream file is optional. However, the skia file is used by both the upstream and downstream bots, and corresponds to the version of skia that both bots build against. In other words, this file always exists and is used on the bots, and thus, it would be an error if the file was missing. Therefore, if you are trying to figure out what the expectations for a test are, and you don't have this file in your checkout, there's a chance you will form the wrong expectations. This means that if you run rebaseline-expectations, you better have this file in your checkout. If rebaseline-test is also trying to update the expectations, it needs the skia file as well, and I think that this implies that you need this even if you're running w/ garden-o-matic. On the other hand, it's obviously annoying to have to do 'update-webkit --chromium' just for this one file, especially when this file is supposed to be empty most of the time. So, I'm not sure what the right thing to do here is. 661755 4 ojan 2012-07-02 15:18:50 -0700 I think this won't generally be a problem in practice. update-expectations during rebaseline-test might not do the 100% optimal thing in a case where a test is listed in both files, but it won't break anything either, i.e. it'll leave the line in the skia test expectations file when it could have removed it. I think this is preferable to requiring anyone rebaselining chromium results to need to get the skia test expectations file. More importantly, most people working in WebKit won't even notice it modifying this file in the chromium repo. 661761 5 dpranke 2012-07-02 15:24:26 -0700 Partially I'm concerned because your change doesn't just affect rebaseline-test, it affects *everything* that tries to use a chromium port, and I'm not sure what all the ramifications of that might be. 661762 6 ojan 2012-07-02 15:27:14 -0700 (In reply to comment #5) > Partially I'm concerned because your change doesn't just affect rebaseline-test, it affects *everything* that tries to use a chromium port, and I'm not sure what all the ramifications of that might be. Your concern is reasonable. As I see it, this is just some of the cost of having this funky skia test expectations file. I'm not too worried because in order to get to a place where you're actually running the tests locally, you'll necessarily have to checkout this file. 661764 7 ojan 2012-07-02 15:27:49 -0700 Would you be happier if we logged a warning? 661769 8 150471 dpranke 2012-07-02 15:36:51 -0700 Comment on attachment 150471 Patch (In reply to comment #7) > Would you be happier if we logged a warning? Probably :) Given that I'm not sure what'll break, and you think we should be okay in practice, I'm okay with landing this and we can see what happens. I just wanted people to understand why this was the way it was. 662332 9 ojan 2012-07-03 10:22:02 -0700 Committed r121784: <http://trac.webkit.org/changeset/121784> 150471 2012-07-02 13:53:42 -0700 2012-07-02 15:36:50 -0700 Patch bug-90400-20120702135341.patch text/plain 3572 ojan U3VidmVyc2lvbiBSZXZpc2lvbjogMTIxNjk5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMTA2ZmNiNmZhM2U5NmVhOTc3MjVhMDc2ZDAwMjJhOThh MzVjMjEyNC4uY2RlZjU4YmY5OGFmZjU5ZjJhNjE3NTQyOWJhYzdlNGFjODQzZDM2NyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIy IEBACiAyMDEyLTA3LTAyICBPamFuIFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKKyAgICAg ICAgTWFrZSB0aGUgc2tpYV90ZXN0X2V4cGVjdGF0aW9ucy50eHQgZmlsZSBvcHRpb25hbC4KKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkwNDAwCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSXQgdXNlZCB0byBi ZSBvcHRpb25hbC4gVGhpcyByZWdyZXNzZWQgYXQgc29tZSBwb2ludC4gSXQncyBpbXBvcnRhbnQg dGhhdCBpdCBiZQorICAgICAgICBvcHRpb25hbCBzbyB0aGF0IHdlYmtpdC1wYXRjaCBjb21tYW5k cyB3b3JrIGluIGEgcHVyZS13ZWJraXQgY2hlY2tvdXQgZm9yIGNocm9taXVtIGJvdHMuCisgICAg ICAgIFNwZWNpZmljYWxseSwgdGhpcyB3YXMgYnJlYWtpbmcgd2Via2l0LXBhdGNoIHJlYmFzZWxp bmUtdGVzdCB3aGVuIGl0IHdvdWxkIGdvIHRvIHVwZGF0ZQorICAgICAgICBUZXN0RXhwZWN0YXRp b25zLgorCisgICAgICAgICogU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJv bWl1bS5weToKKyAgICAgICAgKENocm9taXVtUG9ydC5leHBlY3RhdGlvbnNfZmlsZXMpOgorICAg ICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW1fdW5pdHRl c3QucHk6CisgICAgICAgIChDaHJvbWl1bURyaXZlclRlc3QudGVzdF9leHBlY3RhdGlvbnNfZGlj dCk6CisKKzIwMTItMDctMDIgIE9qYW4gVmFmYWkgIDxvamFuQGNocm9taXVtLm9yZz4KKwogICAg ICAgICBNb3ZlIHJlYmFzZWxpbmUtYWxsIGNvbW1hbmQgZnJvbSB0aGUgZ2FyZGVuaW5nLXNlcnZl ciBkb3duIGludG8gd2Via2l0LXBhdGNoCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD05MDM5NQogCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRw eS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bS5weQppbmRleCAzMmQzNDAxOGM4NWVmZjk4OTBj MjdjYWMyNzVhZTk5ZDJkYzM2YTgwLi4yMjY2ODg5ZDEwNmE2YmMxNzU2NjI3NzRhNjFiNTc1NWU1 NzQ5ZjE0IDEwMDc1NQotLS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9w b3J0L2Nocm9taXVtLnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3Rz L3BvcnQvY2hyb21pdW0ucHkKQEAgLTM0NSw3ICszNDUsMTEgQEAgY2xhc3MgQ2hyb21pdW1Qb3J0 KFdlYktpdFBvcnQpOgogICAgIF0pCiAKICAgICBkZWYgZXhwZWN0YXRpb25zX2ZpbGVzKHNlbGYp OgotICAgICAgICBwYXRocyA9IFtzZWxmLnBhdGhfdG9fdGVzdF9leHBlY3RhdGlvbnNfZmlsZSgp LCBzZWxmLnBhdGhfZnJvbV9jaHJvbWl1bV9iYXNlKCdza2lhJywgJ3NraWFfdGVzdF9leHBlY3Rh dGlvbnMudHh0JyldCisgICAgICAgIHBhdGhzID0gW3NlbGYucGF0aF90b190ZXN0X2V4cGVjdGF0 aW9uc19maWxlKCldCisgICAgICAgIHNraWFfZXhwZWN0YXRpb25zX3BhdGggPSBzZWxmLnBhdGhf ZnJvbV9jaHJvbWl1bV9iYXNlKCdza2lhJywgJ3NraWFfdGVzdF9leHBlY3RhdGlvbnMudHh0JykK KyAgICAgICAgaWYgc2VsZi5fZmlsZXN5c3RlbS5leGlzdHMoc2tpYV9leHBlY3RhdGlvbnNfcGF0 aCk6CisgICAgICAgICAgICBwYXRocy5hcHBlbmQoc2tpYV9leHBlY3RhdGlvbnNfcGF0aCkKKwog ICAgICAgICBidWlsZGVyX25hbWUgPSBzZWxmLmdldF9vcHRpb24oJ2J1aWxkZXJfbmFtZScsICdE VU1NWV9CVUlMREVSX05BTUUnKQogICAgICAgICBpZiBidWlsZGVyX25hbWUgPT0gJ0RVTU1ZX0JV SUxERVJfTkFNRScgb3IgJyhkZXBzKScgaW4gYnVpbGRlcl9uYW1lIG9yIGJ1aWxkZXJfbmFtZSBp biBzZWxmLnRyeV9idWlsZGVyX25hbWVzOgogICAgICAgICAgICAgcGF0aHMuYXBwZW5kKHNlbGYu cGF0aF9mcm9tX2Nocm9taXVtX2Jhc2UoJ3dlYmtpdCcsICd0b29scycsICdsYXlvdXRfdGVzdHMn LCAndGVzdF9leHBlY3RhdGlvbnMudHh0JykpCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtX3VuaXR0ZXN0LnB5IGIvVG9vbHMvU2Ny aXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV91bml0dGVzdC5weQppbmRl eCA4OTgzODBmMmRjZGFhZmQyNzJlMDVlYzc1ZDI0YzcyNjQ2OTcyN2I1Li41Yzc1ZTk3ODhhNmI0 M2VlNWU0NjY4N2IxMDI5NTBlOWY4OTRiNDdmIDEwMDY0NAotLS0gYS9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtX3VuaXR0ZXN0LnB5CisrKyBiL1Rvb2xz L1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW1fdW5pdHRlc3QucHkK QEAgLTE3MCw2ICsxNzAsMTQgQEAgY2xhc3MgQ2hyb21pdW1Ecml2ZXJUZXN0KHVuaXR0ZXN0LlRl c3RDYXNlKToKICAgICAgICAgc2VsZi5kcml2ZXIuX3N0YXJ0KFRydWUsIFtdKQogICAgICAgICBz ZWxmLmFzc2VydEZhbHNlKHNlbGYucG9ydC5fZmlsZXN5c3RlbS5pc2RpcihsYXN0X3RtcGRpcikp CiAKKyAgICBkZWYgdGVzdF9leHBlY3RhdGlvbnNfZGljdChzZWxmKToKKyAgICAgICAgc2VsZi5w b3J0Ll9maWxlc3lzdGVtLndyaXRlX3RleHRfZmlsZSgnL21vY2stY2hlY2tvdXQvTGF5b3V0VGVz dHMvcGxhdGZvcm0vY2hyb21pdW0vVGVzdEV4cGVjdGF0aW9ucycsICdkb3duc3RyZWFtJykKKyAg ICAgICAgc2VsZi5wb3J0Ll9maWxlc3lzdGVtLndyaXRlX3RleHRfZmlsZSgnL21vY2stY2hlY2tv dXQvU291cmNlL1dlYktpdC9jaHJvbWl1bS93ZWJraXQvdG9vbHMvbGF5b3V0X3Rlc3RzL3Rlc3Rf ZXhwZWN0YXRpb25zLnR4dCcsICd1cHN0cmVhbScpCisgICAgICAgIHNlbGYuYXNzZXJ0RXF1YWxz KCdcbicuam9pbihzZWxmLnBvcnQuZXhwZWN0YXRpb25zX2RpY3QoKS52YWx1ZXMoKSksICdkb3du c3RyZWFtXG51cHN0cmVhbScpCisKKyAgICAgICAgc2VsZi5wb3J0Ll9maWxlc3lzdGVtLndyaXRl X3RleHRfZmlsZShzZWxmLnBvcnQucGF0aF9mcm9tX2Nocm9taXVtX2Jhc2UoJ3NraWEnLCAnc2tp YV90ZXN0X2V4cGVjdGF0aW9ucy50eHQnKSwgJ3NraWEnKQorICAgICAgICBzZWxmLmFzc2VydEVx dWFscygnXG4nLmpvaW4oc2VsZi5wb3J0LmV4cGVjdGF0aW9uc19kaWN0KCkudmFsdWVzKCkpLCAn ZG93bnN0cmVhbVxuc2tpYVxudXBzdHJlYW0nKQorCiAKIGNsYXNzIENocm9taXVtUG9ydExvZ2dp bmdUZXN0KGxvZ3Rlc3RpbmcuTG9nZ2luZ1Rlc3RDYXNlKToKIAo=