63838 2011-07-01 13:17:09 -0700 new-run-webkit-tests results does not understand that mac uses test_expectations files 2011-07-01 15:12:04 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 34984 1 eric eric abarth dpranke ojan webkit.review.bot oldest_to_newest 431388 0 eric 2011-07-01 13:17:09 -0700 new-run-webkit-tests does not understand that mac uses test_expectations files When uses_expectations_file is false, these failures appear for mac: test results +fast/canvas/webgl/gl-teximage.html expected actual diff pretty diff +http/tests/cookies/third-party-cookie-relaxing.html expected actual diff pretty diff +inspector/timeline/timeline-layout.html expected actual diff pretty diff +plugins/npp-set-window-called-during-destruction.html expected actual diff pretty diff Yet they're all mentioned in platform/mac/test_expectations.txt: BUGWK58192 : plugins/npp-set-window-called-during-destruction.html = TEXT // Flaky tests when run multi-process BUGWK58192 : fast/dom/frame-loading-via-document-write.html = TEXT PASS BUGWK58192 : http/tests/appcache/fail-on-update-2.html = TEXT PASS BUGWK58192 : http/tests/appcache/fail-on-update.html = TEXT PASS BUGWK58192 : http/tests/inspector/console-websocket-error.html = TEXT PASS BUGWK58192 : fast/canvas/webgl/gl-teximage.html = TEXT PASS BUGWK58192 : fast/frames/flattening/iframe-flattening-offscreen.html = TEXT PASS BUGWK58192 : svg/dom/SVGScriptElement/script-set-href.svg = TEXT PASS Part of the problem is in manager.py: # FIXME: If non-chromium ports start using an expectations file, # we should make this check more robust. results['uses_expectations_file'] = port_obj.name().find('chromium') != -1 however, I dont' think we want to set that bool as it does more than we want for mac. Maybe we can't have our cake and eat it too. WE should probably just move those to the Skipped list for Mac. But platform/mac/test_expetations.txt was a useful tool for the ORWT->NRWT conversion as it's just tests which fail in NRWT but don't in ORWT. 431407 1 dpranke 2011-07-01 13:43:10 -0700 You can probably just replace that line with: port_obj._filesystem.exists(port_obj.path_to_expectations_file()) 431411 2 eric 2011-07-01 13:47:02 -0700 True, we could. I sympathize with what results.html is doing. It's being told not to expect test_expectations, and then I'm expecting it to filter out test_expetations-base failures. I suspect we should just move these all to the Skipped lists. 431426 3 99520 eric 2011-07-01 14:07:49 -0700 Created attachment 99520 Patch 431430 4 eric 2011-07-01 14:08:52 -0700 Thank you for the suggestion Dirk! 431434 5 99520 dpranke 2011-07-01 14:12:06 -0700 Comment on attachment 99520 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99520&action=review > Tools/Scripts/webkitpy/layout_tests/port/base.py:718 > + I probably wouldn't have introduced a separate function just for this, since it's currently only called in one place ... > Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py:264 > + def test_uses_test_expectations_file(self): In addition to doing the tests on the base class, I usually try to add a test in the port_testcase.py class that is then run on every subclass, to ensure the subclass is implementing/overridding the routine as expected. 431435 6 ojan 2011-07-01 14:14:43 -0700 It seems to me that we either need to use an expectations file with all the complexity it involves or just used Skipped lists. I don't see a reasonable middle ground off the top of my head. Doesn't this patch just make it so that Mac also uses an expectations file? I think that's fine, but I have heard strong resistance to this complexity from maintainers of the Apple port. I think this code change is correct regardless, but it still leaves open the question of whether Apple's port should have an expectations file. 431437 7 abarth 2011-07-01 14:18:12 -0700 > It seems to me that we either need to use an expectations file with all the complexity it involves or just used Skipped lists. I don't see a reasonable middle ground off the top of my head. We have have a less complex version of the test_expectations file. > Doesn't this patch just make it so that Mac also uses an expectations file? I think that's fine, but I have heard strong resistance to this complexity from maintainers of the Apple port. Nope. It uses both. > I think this code change is correct regardless, but it still leaves open the question of whether Apple's port should have an expectations file. All the ports should work the same way. We'll find something that works for everyone. 431440 8 ojan 2011-07-01 14:19:23 -0700 (In reply to comment #7) > > It seems to me that we either need to use an expectations file with all the complexity it involves or just used Skipped lists. I don't see a reasonable middle ground off the top of my head. > > We have have a less complex version of the test_expectations file. In what way? Just because it's shorter? The downside is that you then need to surface all the expectations information into the results.html page. 431452 9 abarth 2011-07-01 14:30:29 -0700 > In what way? Just because it's shorter? The downside is that you then need to surface all the expectations information into the results.html page. Sorry, I mean to say "we can have have a less complex version". The test_expectations.txt file is more complex than it needs to be for the job it does. 431458 10 dpranke 2011-07-01 14:38:44 -0700 (In reply to comment #9) > > In what way? Just because it's shorter? The downside is that you then need to surface all the expectations information into the results.html page. > > Sorry, I mean to say "we can have have a less complex version". The test_expectations.txt file is more complex than it needs to be for the job it does. Are you saying that the test_expectations.txt file is attempting to do more than it should be doing (has too much functionality), or that the existing functionality could be implemented more simply? Examples for either? 431459 11 abarth 2011-07-01 14:40:59 -0700 This the wrong forum for this discussion. 431482 12 99520 webkit.review.bot 2011-07-01 15:11:59 -0700 Comment on attachment 99520 Patch Clearing flags on attachment: 99520 Committed r90285: <http://trac.webkit.org/changeset/90285> 431483 13 webkit.review.bot 2011-07-01 15:12:04 -0700 All reviewed patches have been landed. Closing bug. 99520 2011-07-01 14:07:49 -0700 2011-07-01 15:11:59 -0700 Patch bug-63838-20110701140748.patch text/plain 5794 eric U3VidmVyc2lvbiBSZXZpc2lvbjogOTAyNjEKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCBjMDhiNGUwOTNkZWFmM2RlZWNlZDdlMmMzNWY3ZjMwZWJl YmUyOTkxLi5lYTQ2YThhN2I3YTBmYWRiYWY1MmJlYWIyMGM1NzBhYzg3MGZmOTAxIDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTEtMDctMDEgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQub3JnPgorCisgICAgICAg IG5ldy1ydW4td2Via2l0LXRlc3RzIHJlc3VsdHMgZG9lcyBub3QgdW5kZXJzdGFuZCB0aGF0IG1h YyB1c2VzIHRlc3RfZXhwZWN0YXRpb25zIGZpbGVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MzgzOAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFdlJ3ZlIGRlY2lkZWQgdG8gImVuYWJsZSIgdGVzdF9leHBl Y3RhdGlvbnMgZm9yIHRoZSBNYWMgcG9ydCBmb3Igbm93LgorICAgICAgICBJdCBtYWtlcyB0aGUg cmVzdWx0cy5odG1sIHJlc3VsdHMgbXVjaCBtb3JlIGNvbmZ1c2luZyB0byByZWFkLCBidXQgYXQg bGVhc3QgdGhleSdyZQorICAgICAgICBubyBsb25nZXIgbHlpbmcgdG8gdXMuCisKKyAgICAgICAg KiBTY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9sYXlvdXRfcGFja2FnZS9tYW5hZ2VyLnB5 OgorICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZS5weToK KyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Jhc2VfdW5pdHRl c3QucHk6CisKIDIwMTEtMDctMDEgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KIAog ICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBBZGxlci4KZGlmZiAtLWdpdCBhL1Rvb2xzL1Njcmlw dHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL2xheW91dF9wYWNrYWdlL21hbmFnZXIucHkgYi9Ub29s cy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9sYXlvdXRfcGFja2FnZS9tYW5hZ2VyLnB5 CmluZGV4IGUzZmEwYTZmMjRkZjJjODRlYjgzZDFhZjE5MDllMDQyNjMxZmM4NzUuLjM0YzM1M2Vi NzRiY2JhZDAyZWJlYWY0YTNjM2M1ZTkzZmEzYTBjZWYgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1Njcmlw dHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL2xheW91dF9wYWNrYWdlL21hbmFnZXIucHkKKysrIGIv VG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvbGF5b3V0X3BhY2thZ2UvbWFuYWdl ci5weQpAQCAtMjA2LDkgKzIwNiw3IEBAIGRlZiBzdW1tYXJpemVfcmVzdWx0cyhwb3J0X29iaiwg ZXhwZWN0YXRpb25zLCByZXN1bHRfc3VtbWFyeSwgcmV0cnlfc3VtbWFyeSwgdGVzCiAgICAgcmVz dWx0c1snbnVtX3Bhc3NlcyddID0gbnVtX3Bhc3NlcwogICAgIHJlc3VsdHNbJ251bV9mbGFreSdd ID0gbnVtX2ZsYWt5CiAgICAgcmVzdWx0c1snbnVtX3JlZ3Jlc3Npb25zJ10gPSBudW1fcmVncmVz c2lvbnMKLSAgICAjIEZJWE1FOiBJZiBub24tY2hyb21pdW0gcG9ydHMgc3RhcnQgdXNpbmcgYW4g ZXhwZWN0YXRpb25zIGZpbGUsCi0gICAgIyB3ZSBzaG91bGQgbWFrZSB0aGlzIGNoZWNrIG1vcmUg cm9idXN0LgotICAgIHJlc3VsdHNbJ3VzZXNfZXhwZWN0YXRpb25zX2ZpbGUnXSA9IHBvcnRfb2Jq Lm5hbWUoKS5maW5kKCdjaHJvbWl1bScpICE9IC0xCisgICAgcmVzdWx0c1sndXNlc19leHBlY3Rh dGlvbnNfZmlsZSddID0gcG9ydF9vYmoudXNlc190ZXN0X2V4cGVjdGF0aW9uc19maWxlKCkKICAg ICByZXN1bHRzWydsYXlvdXRfdGVzdHNfZGlyJ10gPSBwb3J0X29iai5sYXlvdXRfdGVzdHNfZGly KCkKICAgICByZXN1bHRzWydoYXNfd2RpZmYnXSA9IHBvcnRfb2JqLndkaWZmX2F2YWlsYWJsZSgp CiAgICAgcmVzdWx0c1snaGFzX3ByZXR0eV9wYXRjaCddID0gcG9ydF9vYmoucHJldHR5X3BhdGNo X2F2YWlsYWJsZSgpCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90 ZXN0cy9wb3J0L2Jhc2UucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9w b3J0L2Jhc2UucHkKaW5kZXggMGQyMzZhYWMwZmQwZDQ3MTQxOTYyMmFkZmZiYWI1ZGUxMzE0MGZi Yy4uNzMyYjBkM2JhZTAzNDRkZTQ4YmUzNGM3ZjJkY2M1ZDgwYWEzOTk5MSAxMDA3NTUKLS0tIGEv VG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9iYXNlLnB5CisrKyBiL1Rv b2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZS5weQpAQCAtNzExLDYg KzcxMSwxMSBAQCBjbGFzcyBQb3J0KG9iamVjdCk6CiAgICAgICAgICdzbm93bGVvcGFyZCcgc2hv dWxkIHByZWNlZGUgJ2xlb3BhcmQnKS4iIiIKICAgICAgICAgcmFpc2UgTm90SW1wbGVtZW50ZWRF cnJvcgogCisgICAgZGVmIHVzZXNfdGVzdF9leHBlY3RhdGlvbnNfZmlsZShzZWxmKToKKyAgICAg ICAgIyBUaGlzIGlzIGRpZmZlcmVudCBmcm9tIGNoZWNraW5nIHRlc3RfZXhwZWN0YXRpb25zKCkg aXMgTm9uZSwgYmVjYXVzZQorICAgICAgICAjIHNvbWUgcG9ydHMgaGF2ZSBTa2lwcGVkIGZpbGVz IHdoaWNoIGFyZSByZXR1cm5lZCBhcyBwYXJ0IG9mIHRlc3RfZXhwZWN0YXRpb25zKCkuCisgICAg ICAgIHJldHVybiBzZWxmLl9maWxlc3lzdGVtLmV4aXN0cyhzZWxmLnBhdGhfdG9fdGVzdF9leHBl Y3RhdGlvbnNfZmlsZSgpKQorCiAgICAgZGVmIHRlc3RfZXhwZWN0YXRpb25zKHNlbGYpOgogICAg ICAgICAiIiJSZXR1cm5zIHRoZSB0ZXN0IGV4cGVjdGF0aW9ucyBmb3IgdGhpcyBwb3J0LgogCmRp ZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Jhc2Vf dW5pdHRlc3QucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Jh c2VfdW5pdHRlc3QucHkKaW5kZXggY2ViMzNlODhmMTViYzgzMzk0OTFmZDg5YjdiZDExOWEyZDM3 MzY4Yy4uYTRiMjJkNmUzOTQ5YWNjMDNkOTkxY2U1YjhiNGU1MDcxZTYzZTBhYSAxMDA2NDQKLS0t IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9iYXNlX3VuaXR0ZXN0 LnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZV91 bml0dGVzdC5weQpAQCAtMjA3LDggKzIwNyw3IEBAIGNsYXNzIFBvcnRUZXN0KHVuaXR0ZXN0LlRl c3RDYXNlKToKICAgICAgICAgICAgIHByZWZpeCA9ICJmaWxlOi8vIgogICAgICAgICAgICAgcGF0 aCA9IHRlc3RfZmlsZQogCi0gICAgICAgIHNlbGYuYXNzZXJ0RXF1YWwocG9ydC5maWxlbmFtZV90 b191cmkodGVzdF9maWxlKSwKLSAgICAgICAgICAgICAgICAgICAgICAgICBhYnNwYXRoX3RvX3Vy aSh0ZXN0X2ZpbGUpKQorICAgICAgICBzZWxmLmFzc2VydEVxdWFsKHBvcnQuZmlsZW5hbWVfdG9f dXJpKHRlc3RfZmlsZSksIGFic3BhdGhfdG9fdXJpKHRlc3RfZmlsZSkpCiAKICAgICBkZWYgdGVz dF9nZXRfb3B0aW9uX19zZXQoc2VsZik6CiAgICAgICAgIG9wdGlvbnMsIGFyZ3MgPSBvcHRwYXJz ZS5PcHRpb25QYXJzZXIoKS5wYXJzZV9hcmdzKFtdKQpAQCAtMjM0LDggKzIzMyw3IEBAIGNsYXNz IFBvcnRUZXN0KHVuaXR0ZXN0LlRlc3RDYXNlKToKIAogICAgIGRlZiB0ZXN0X2FkZGl0aW9uYWxf cGxhdGZvcm1fZGlyZWN0b3J5KHNlbGYpOgogICAgICAgICBmaWxlc3lzdGVtID0gTW9ja0ZpbGVT eXN0ZW0oKQotICAgICAgICBvcHRpb25zLCBhcmdzID0gb3B0cGFyc2UuT3B0aW9uUGFyc2VyKCku cGFyc2VfYXJncyhbXSkKLSAgICAgICAgcG9ydCA9IGJhc2UuUG9ydChwb3J0X25hbWU9J2Zvbycs IGZpbGVzeXN0ZW09ZmlsZXN5c3RlbSwgb3B0aW9ucz1vcHRpb25zKQorICAgICAgICBwb3J0ID0g YmFzZS5Qb3J0KHBvcnRfbmFtZT0nZm9vJywgZmlsZXN5c3RlbT1maWxlc3lzdGVtKQogICAgICAg ICBwb3J0LmJhc2VsaW5lX3NlYXJjaF9wYXRoID0gbGFtYmRhOiBbJ0xheW91dFRlc3RzL3BsYXRm b3JtL2ZvbyddCiAgICAgICAgIGxheW91dF90ZXN0X2RpciA9IHBvcnQubGF5b3V0X3Rlc3RzX2Rp cigpCiAgICAgICAgIHRlc3RfZmlsZSA9IGZpbGVzeXN0ZW0uam9pbihsYXlvdXRfdGVzdF9kaXIs ICdmYXN0JywgJ3Rlc3QuaHRtbCcpCkBAIC0yNDcsNyArMjQ1LDcgQEAgY2xhc3MgUG9ydFRlc3Qo dW5pdHRlc3QuVGVzdENhc2UpOgogICAgICAgICBzZWxmLmFzc2VydEVxdWFsKHBvcnQuYmFzZWxp bmVfcGF0aCgpLCAnTGF5b3V0VGVzdHMvcGxhdGZvcm0vZm9vJykKIAogICAgICAgICAjIFNpbXBs ZSBhZGRpdGlvbmFsIHBsYXRmb3JtIGRpcmVjdG9yeQotICAgICAgICBvcHRpb25zLmFkZGl0aW9u YWxfcGxhdGZvcm1fZGlyZWN0b3J5ID0gWycvdG1wL2xvY2FsLWJhc2VsaW5lcyddCisgICAgICAg IHBvcnQuX29wdGlvbnMuYWRkaXRpb25hbF9wbGF0Zm9ybV9kaXJlY3RvcnkgPSBbJy90bXAvbG9j YWwtYmFzZWxpbmVzJ10KICAgICAgICAgZmlsZXN5c3RlbS5maWxlcyA9IHsKICAgICAgICAgICAg ICcvdG1wL2xvY2FsLWJhc2VsaW5lcy9mYXN0L3Rlc3QtZXhwZWN0ZWQudHh0JzogJ2ZvbycsCiAg ICAgICAgIH0KQEAgLTI1NywxMiArMjU1LDIxIEBAIGNsYXNzIFBvcnRUZXN0KHVuaXR0ZXN0LlRl c3RDYXNlKToKICAgICAgICAgc2VsZi5hc3NlcnRFcXVhbChwb3J0LmJhc2VsaW5lX3BhdGgoKSwg Jy90bXAvbG9jYWwtYmFzZWxpbmVzJykKIAogICAgICAgICAjIE11bHRpcGxlIGFkZGl0aW9uYWwg cGxhdGZvcm0gZGlyZWN0b3JpZXMKLSAgICAgICAgb3B0aW9ucy5hZGRpdGlvbmFsX3BsYXRmb3Jt X2RpcmVjdG9yeSA9IFsnL2ZvbycsICcvdG1wL2xvY2FsLWJhc2VsaW5lcyddCisgICAgICAgIHBv cnQuX29wdGlvbnMuYWRkaXRpb25hbF9wbGF0Zm9ybV9kaXJlY3RvcnkgPSBbJy9mb28nLCAnL3Rt cC9sb2NhbC1iYXNlbGluZXMnXQogICAgICAgICBzZWxmLmFzc2VydEVxdWFsKAogICAgICAgICAg ICAgcG9ydC5leHBlY3RlZF9iYXNlbGluZXModGVzdF9maWxlLCAnLnR4dCcpLAogICAgICAgICAg ICAgWygnL3RtcC9sb2NhbC1iYXNlbGluZXMnLCAnZmFzdC90ZXN0LWV4cGVjdGVkLnR4dCcpXSkK ICAgICAgICAgc2VsZi5hc3NlcnRFcXVhbChwb3J0LmJhc2VsaW5lX3BhdGgoKSwgJy9mb28nKQog CisgICAgZGVmIHRlc3RfdXNlc190ZXN0X2V4cGVjdGF0aW9uc19maWxlKHNlbGYpOgorICAgICAg ICBmaWxlc3lzdGVtID0gTW9ja0ZpbGVTeXN0ZW0oKQorICAgICAgICBwb3J0ID0gYmFzZS5Qb3J0 KHBvcnRfbmFtZT0nZm9vJywgZmlsZXN5c3RlbT1maWxlc3lzdGVtKQorICAgICAgICBwb3J0LnBh dGhfdG9fdGVzdF9leHBlY3RhdGlvbnNfZmlsZSA9IGxhbWJkYTogJy9tb2NrL3Rlc3RfZXhwZWN0 YXRpb25zLnR4dCcKKyAgICAgICAgc2VsZi5hc3NlcnRGYWxzZShwb3J0LnVzZXNfdGVzdF9leHBl Y3RhdGlvbnNfZmlsZSgpKQorICAgICAgICBwb3J0Ll9maWxlc3lzdGVtID0gTW9ja0ZpbGVTeXN0 ZW0oeycvbW9jay90ZXN0X2V4cGVjdGF0aW9ucy50eHQnOiAnJ30pCisgICAgICAgIHNlbGYuYXNz ZXJ0VHJ1ZShwb3J0LnVzZXNfdGVzdF9leHBlY3RhdGlvbnNfZmlsZSgpKQorCisKIGNsYXNzIFZp cnR1YWxUZXN0KHVuaXR0ZXN0LlRlc3RDYXNlKToKICAgICAiIiJUZXN0cyB0aGF0IHZhcmlvdXMg bWV0aG9kcyBleHBlY3RlZCB0byBiZSB2aXJ0dWFsIGFyZS4iIiIKICAgICBkZWYgYXNzZXJ0Vmly dHVhbChzZWxmLCBtZXRob2QsICphcmdzLCAqKmt3YXJncyk6Cg==