46278 2010-09-22 10:39:55 -0700 NRWT doesn't respect config set with set-webkit-configuration 2010-09-26 20:40:39 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 mihaip dpranke abarth caseq commit-queue ctguil dpranke eric senorblanco tony oldest_to_newest 283044 0 mihaip 2010-09-22 10:39:55 -0700 Even though I ran set-webkit-configuration --debug a while back (and build-webkit --chromium respects it), NRWT kept looking for the DRT binary in the Release directory until I passed an explicit --debug flag to it. I suspect http://trac.webkit.org/changeset/68008 (unintentionally?) triggered this behavior change. 283125 1 dpranke 2010-09-22 12:04:13 -0700 Odd ... I thought I had a test for this. Looking into it now; it's certainly possible that that change broke this. 284113 2 mihaip 2010-09-23 17:02:17 -0700 +Adam, since he reviewed r68008. There was a change to chromium.py which is responsible for this: http://trac.webkit.org/changeset/68008/trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py Was there a reason why ChromiumPort now sets the default configuration to Release in it constructor? There is code that runs later (_set_up_derived_options in run_webkit_tests.py) which would do the similar initialization: if not options.configuration: options.configuration = port_obj.default_configuration() 284128 3 mihaip 2010-09-23 17:21:36 -0700 (In reply to comment #2) > Was there a reason why ChromiumPort now sets the default configuration to Release in it constructor? I now see that that code was always there, just duplicated in Mac/Linux/Windows subclasses of ChromiumPort. However, if options and not hasattr(options, 'configuration'): options.configuration = 'Release' Whereas now it's more aggressive: if (options and (not hasattr(options, 'configuration') or options.configuration is None)): options.configuration = 'Release' options.configuration is defined by parse_args in run_webkit_tests.py, it's just set to None. Any objection to reverting the options.configuration check to the way it was before? 284179 4 dpranke 2010-09-23 18:50:36 -0700 No objection. This code was accidentally broken. I am in the middle of fixing it properly and writing better unit tests so that it doesn't break again. This turns out to involve a fair amount of yak-shaving, but I should have a patch this evening. 284216 5 mihaip 2010-09-23 22:10:09 -0700 (In reply to comment #4) > I am in the middle of fixing it properly and writing better unit tests so that it doesn't break again. This turns out to involve a fair amount of yak-shaving, but I should have a patch this evening. I was looking into the "right" way of fixing it this afternoon, and it seemed like having the ChromiumPort constructor call self.default_configuration() instead of hardcoding 'Release' would be better, but I wasn't sure what the implications of that would be (for starters, the unit tests didn't pass, since they didn't mock out the configuration reading so were getting it from the filesystem). 284431 6 68690 mihaip 2010-09-24 09:25:47 -0700 Created attachment 68690 Patch 284432 7 mihaip 2010-09-24 09:27:36 -0700 +Tony, since I see he rolled back Dirk's quick fix since it broke NRWT on the canary bots (http://trac.webkit.org/changeset/68234). What exactly broke, and how can I make sure my patch doesn't break things too? 284468 8 tony 2010-09-24 10:04:27 -0700 (In reply to comment #7) > +Tony, since I see he rolled back Dirk's quick fix since it broke NRWT on the canary bots (http://trac.webkit.org/changeset/68234). What exactly broke, and how can I make sure my patch doesn't break things too? It failed because of: import pdb pdb.set_trace() Which put the interpreter into interactive mode. 284490 9 68690 commit-queue 2010-09-24 10:22:59 -0700 Comment on attachment 68690 Patch Clearing flags on attachment: 68690 Committed r68268: <http://trac.webkit.org/changeset/68268> 284491 10 commit-queue 2010-09-24 10:23:05 -0700 All reviewed patches have been landed. Closing bug. 284606 11 dpranke 2010-09-24 12:14:16 -0700 (In reply to comment #6) > Created an attachment (id=68690) [details] > Patch Yup, that'll work, although there's a lot more scaffolding than I would hope to have and it still doesn't test all the way through to make sure that every port is actually hitting the config file to pull the default (i.e., if ChromiumPort accidentally (?) defines a different default_configuration() we won't notice it in these tests). So, I'll keep hacking on the "right" fix but thanks for taking care of this for now. -- Dirk 285107 12 caseq 2010-09-26 06:09:06 -0700 This change apparently broke nacl_ui_tests in chromium: http://build.chromium.org/buildbot/waterfall/builders/NACL%20Tests/builds/17999 285247 13 mihaip 2010-09-26 19:20:28 -0700 (In reply to comment #12) > This change apparently broke nacl_ui_tests in chromium: > > http://build.chromium.org/buildbot/waterfall/builders/NACL%20Tests/builds/17999 Thanks for letting me know. I've put up a fix at bug 46602. 285263 14 mihaip 2010-09-26 20:40:39 -0700 (In reply to comment #13) > Thanks for letting me know. I've put up a fix at bug 46602. Landed as http://trac.webkit.org/changeset/68365. 68690 2010-09-24 09:25:47 -0700 2010-09-24 10:22:59 -0700 Patch bug-46278-20100924092546.patch text/plain 5851 mihaip ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCBhYjJkMWY2ZTRlM2RlYTQyMzlhZDUyY2I5NGM2MmUwMDk1MjE4NTQyLi5hMDY3YWJk OWVhMjc1ZDJkYzk3MGNmMjhjNGY2NzIzMWQxMTA0OWE4IDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAK KzIwMTAtMDktMjMgIE1paGFpIFBhcnBhcml0YSAgPG1paGFpcEBjaHJvbWl1bS5vcmc+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTlJXVCBkb2Vzbid0 IHJlc3BlY3QgY29uZmlnIHNldCB3aXRoIHNldC13ZWJraXQtY29uZmlndXJhdGlvbgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDYyNzgKKyAgICAgICAg CisgICAgICAgIFVzZSBQb3J0LmRlZmF1bHRfY29uZmlndXJhdGlvbigpIGluc3RlYWQgb2YgaGFy ZGNvZGluZyBSZWxlYXNlIGluCisgICAgICAgIENocm9taXVtUG9ydCBjb25maWd1cmF0aW9uIGlu aXRpYWxpemF0aW9uLCBzbyB0aGF0IHdlIHN0aWxsIGluaGVyaXQgdGhlCisgICAgICAgIGNvbmZp Z3VyYXRpb24gc2V0IGJ5IHNldC13ZWJraXQtY29uZmlndXJhdGlvbi4KKworICAgICAgICAqIFNj cmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW0ucHk6CisgICAgICAgICog U2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV91bml0dGVzdC5weToK KwogMjAxMC0wOS0yNCAgQ2hhbmcgU2h1ICA8Y2hhbmcuc2h1QG5va2lhLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBDc2FiYSBPc3p0cm9nb27DoWMuCmRpZmYgLS1naXQgYS9XZWJLaXRUb29s cy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtLnB5IGIvV2ViS2l0 VG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bS5weQppbmRl eCA0ZjNkNzM0NWU1Mzk5YTEyZTJmMTg3MWIyNDFmY2NlNzc5NDdjZGUzLi5hNzI2MjdhMTZhN2Zh NTRjNDg4M2M0MzkyOTVhNzAzZTk1ZjNiOGQ3IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9TY3Jp cHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2Nocm9taXVtLnB5CisrKyBiL1dlYktpdFRv b2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW0ucHkKQEAgLTgz LDEyICs4MywxMiBAQCBjbGFzcyBDaHJvbWl1bVBvcnQoYmFzZS5Qb3J0KToKICAgICAiIiJBYnN0 cmFjdCBiYXNlIGNsYXNzIGZvciBDaHJvbWl1bSBpbXBsZW1lbnRhdGlvbnMgb2YgdGhlIFBvcnQg Y2xhc3MuIiIiCiAKICAgICBkZWYgX19pbml0X18oc2VsZiwgKiprd2FyZ3MpOgorICAgICAgICBi YXNlLlBvcnQuX19pbml0X18oc2VsZiwgKiprd2FyZ3MpCiAgICAgICAgIGlmICdvcHRpb25zJyBp biBrd2FyZ3M6CiAgICAgICAgICAgICBvcHRpb25zID0ga3dhcmdzWydvcHRpb25zJ10KICAgICAg ICAgICAgIGlmIChvcHRpb25zIGFuZCAobm90IGhhc2F0dHIob3B0aW9ucywgJ2NvbmZpZ3VyYXRp b24nKSBvcgogICAgICAgICAgICAgICAgICAgICAgICAgICAgICBvcHRpb25zLmNvbmZpZ3VyYXRp b24gaXMgTm9uZSkpOgotICAgICAgICAgICAgICAgIG9wdGlvbnMuY29uZmlndXJhdGlvbiA9ICdS ZWxlYXNlJwotICAgICAgICBiYXNlLlBvcnQuX19pbml0X18oc2VsZiwgKiprd2FyZ3MpCisgICAg ICAgICAgICAgICAgb3B0aW9ucy5jb25maWd1cmF0aW9uID0gc2VsZi5kZWZhdWx0X2NvbmZpZ3Vy YXRpb24oKQogICAgICAgICBzZWxmLl9jaHJvbWl1bV9iYXNlX2RpciA9IE5vbmUKIAogICAgIGRl ZiBiYXNlbGluZV9wYXRoKHNlbGYpOgpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV91bml0dGVzdC5weSBiL1dlYktpdFRv b2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvY2hyb21pdW1fdW5pdHRlc3Qu cHkKaW5kZXggMWNmYmRlMTc3NGFhZTNlODgzMzdkNTMyYzk4ZjA5ZTFjNGM1OGM2MC4uYTRhOWVh NjA2NmM2ZmEzMDUxOTQ2ZmUyYWRiNmI2N2Q1YzNjYjAxNyAxMDA2NDQKLS0tIGEvV2ViS2l0VG9v bHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9jaHJvbWl1bV91bml0dGVzdC5w eQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L2No cm9taXVtX3VuaXR0ZXN0LnB5CkBAIC04MywxOSArODMsMzkgQEAgY2xhc3MgQ2hyb21pdW1Ecml2 ZXJUZXN0KHVuaXR0ZXN0LlRlc3RDYXNlKToKICAgICAgICAgc2VsZi5kcml2ZXIuX3Byb2Muc3Rk b3V0LnJlYWRsaW5lID0gbW9ja19yZWFkbGluZQogICAgICAgICBzZWxmLl9hc3NlcnRfd3JpdGVf Y29tbWFuZF9hbmRfcmVhZF9saW5lKGV4cGVjdGVkX2NyYXNoPVRydWUpCiAKKworY2xhc3MgQ2hy b21pdW1Qb3J0VGVzdCh1bml0dGVzdC5UZXN0Q2FzZSk6CisgICAgY2xhc3MgVGVzdE1hY1BvcnQo Y2hyb21pdW1fbWFjLkNocm9taXVtTWFjUG9ydCk6CisgICAgICAgIGRlZiBfX2luaXRfXyhzZWxm LCBvcHRpb25zKToKKyAgICAgICAgICAgIGNocm9taXVtX21hYy5DaHJvbWl1bU1hY1BvcnQuX19p bml0X18oc2VsZiwKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgcG9ydF9uYW1lPSd0ZXN0LXBvcnQnLAorICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBvcHRpb25zPW9wdGlvbnMpCisKKyAgICAgICAgZGVmIGRl ZmF1bHRfY29uZmlndXJhdGlvbihzZWxmKToKKyAgICAgICAgICAgIHNlbGYuZGVmYXVsdF9jb25m aWd1cmF0aW9uX2NhbGxlZCA9IFRydWUKKyAgICAgICAgICAgIHJldHVybiAnZGVmYXVsdCcKKwor ICAgIGNsYXNzIFRlc3RMaW51eFBvcnQoY2hyb21pdW1fbGludXguQ2hyb21pdW1MaW51eFBvcnQp OgorICAgICAgICBkZWYgX19pbml0X18oc2VsZiwgb3B0aW9ucyk6CisgICAgICAgICAgICBjaHJv bWl1bV9saW51eC5DaHJvbWl1bUxpbnV4UG9ydC5fX2luaXRfXyhzZWxmLAorICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcG9ydF9uYW1lPSd0ZXN0 LXBvcnQnLAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgb3B0aW9ucz1vcHRpb25zKQorCisgICAgICAgIGRlZiBkZWZhdWx0X2NvbmZpZ3VyYXRp b24oc2VsZik6CisgICAgICAgICAgICBzZWxmLmRlZmF1bHRfY29uZmlndXJhdGlvbl9jYWxsZWQg PSBUcnVlCisgICAgICAgICAgICByZXR1cm4gJ2RlZmF1bHQnCisKICAgICBkZWYgdGVzdF9wYXRo X3RvX2ltYWdlX2RpZmYoc2VsZik6CiAgICAgICAgIGNsYXNzIE1vY2tPcHRpb25zOgogICAgICAg ICAgICAgZGVmIF9faW5pdF9fKHNlbGYpOgogICAgICAgICAgICAgICAgIHNlbGYudXNlX2RydCA9 IFRydWUKIAotICAgICAgICBwb3J0ID0gY2hyb21pdW1fbGludXguQ2hyb21pdW1MaW51eFBvcnQo cG9ydF9uYW1lPSd0ZXN0LXBvcnQnLAotICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgb3B0aW9ucz1Nb2NrT3B0aW9ucygpKQorICAgICAgICBwb3J0ID0gQ2hy b21pdW1Qb3J0VGVzdC5UZXN0TGludXhQb3J0KG9wdGlvbnM9TW9ja09wdGlvbnMoKSkKICAgICAg ICAgc2VsZi5hc3NlcnRUcnVlKHBvcnQuX3BhdGhfdG9faW1hZ2VfZGlmZigpLmVuZHN3aXRoKAot ICAgICAgICAgICAgJy9vdXQvUmVsZWFzZS9JbWFnZURpZmYnKSkKLSAgICAgICAgcG9ydCA9IGNo cm9taXVtX21hYy5DaHJvbWl1bU1hY1BvcnQocG9ydF9uYW1lPSd0ZXN0LXBvcnQnLAotICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBvcHRpb25zPU1vY2tPcHRpb25z KCkpCisgICAgICAgICAgICAnL291dC9kZWZhdWx0L0ltYWdlRGlmZicpLCBtc2c9cG9ydC5fcGF0 aF90b19pbWFnZV9kaWZmKCkpCisgICAgICAgIHBvcnQgPSBDaHJvbWl1bVBvcnRUZXN0LlRlc3RN YWNQb3J0KG9wdGlvbnM9TW9ja09wdGlvbnMoKSkKICAgICAgICAgc2VsZi5hc3NlcnRUcnVlKHBv cnQuX3BhdGhfdG9faW1hZ2VfZGlmZigpLmVuZHN3aXRoKAotICAgICAgICAgICAgJy94Y29kZWJ1 aWxkL1JlbGVhc2UvSW1hZ2VEaWZmJykpCisgICAgICAgICAgICAnL3hjb2RlYnVpbGQvZGVmYXVs dC9JbWFnZURpZmYnKSkKICAgICAgICAgIyBGSVhNRTogRmlndXJlIG91dCBob3cgdGhpcyBpcyBn b2luZyB0byB3b3JrIG9uIFdpbmRvd3MuCiAgICAgICAgICNwb3J0ID0gY2hyb21pdW1fd2luLkNo cm9taXVtV2luUG9ydCgndGVzdC1wb3J0Jywgb3B0aW9ucz1Nb2NrT3B0aW9ucygpKQogCkBAIC0x MDQsOCArMTI0LDcgQEAgY2xhc3MgQ2hyb21pdW1Ecml2ZXJUZXN0KHVuaXR0ZXN0LlRlc3RDYXNl KToKICAgICAgICAgICAgIGRlZiBfX2luaXRfXyhzZWxmKToKICAgICAgICAgICAgICAgICBzZWxm LnVzZV9kcnQgPSBUcnVlCiAKLSAgICAgICAgcG9ydCA9IGNocm9taXVtX2xpbnV4LkNocm9taXVt TGludXhQb3J0KHBvcnRfbmFtZT0ndGVzdC1wb3J0JywKLSAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgIG9wdGlvbnM9TW9ja09wdGlvbnMoKSkKKyAgICAgICAg cG9ydCA9IENocm9taXVtUG9ydFRlc3QuVGVzdExpbnV4UG9ydChvcHRpb25zPU1vY2tPcHRpb25z KCkpCiAKICAgICAgICAgZmFrZV90ZXN0ID0gb3MucGF0aC5qb2luKHBvcnQubGF5b3V0X3Rlc3Rz X2RpcigpLCAiZmFzdC9qcy9ub3QtZ29vZC5qcyIpCiAKQEAgLTExOCw1ICsxMzcsMjQgQEAgREVG RVIgTElOVVggV0lOIDogZmFzdC9qcy92ZXJ5LWdvb2QuanMgPSBUSU1FT1VUIFBBU1MiIiIKICAg ICAgICAgc2tpcHBlZF90ZXN0cyA9IHBvcnQuc2tpcHBlZF9sYXlvdXRfdGVzdHMoZXh0cmFfdGVz dF9maWxlcz1bZmFrZV90ZXN0LCBdKQogICAgICAgICBzZWxmLmFzc2VydFRydWUoImZhc3QvanMv bm90LWdvb2QuanMiIGluIHNraXBwZWRfdGVzdHMpCiAKKyAgICBkZWYgdGVzdF9kZWZhdWx0X2Nv bmZpZ3VyYXRpb24oc2VsZik6CisgICAgICAgIGNsYXNzIEVtcHR5T3B0aW9uczoKKyAgICAgICAg ICAgIGRlZiBfX2luaXRfXyhzZWxmKToKKyAgICAgICAgICAgICAgICBwYXNzCisKKyAgICAgICAg b3B0aW9ucyA9IEVtcHR5T3B0aW9ucygpCisgICAgICAgIHBvcnQgPSBDaHJvbWl1bVBvcnRUZXN0 LlRlc3RMaW51eFBvcnQob3B0aW9ucykKKyAgICAgICAgc2VsZi5hc3NlcnRFcXVhbHMob3B0aW9u cy5jb25maWd1cmF0aW9uLCAnZGVmYXVsdCcpCisgICAgICAgIHNlbGYuYXNzZXJ0VHJ1ZShwb3J0 LmRlZmF1bHRfY29uZmlndXJhdGlvbl9jYWxsZWQpCisKKyAgICAgICAgY2xhc3MgT3B0aW9uc1dp dGhVbnNldENvbmZpZ3VyYXRpb246CisgICAgICAgICAgICBkZWYgX19pbml0X18oc2VsZik6Cisg ICAgICAgICAgICAgICAgc2VsZi5jb25maWd1cmF0aW9uID0gTm9uZQorCisgICAgICAgIG9wdGlv bnMgPSBPcHRpb25zV2l0aFVuc2V0Q29uZmlndXJhdGlvbigpCisgICAgICAgIHBvcnQgPSBDaHJv bWl1bVBvcnRUZXN0LlRlc3RMaW51eFBvcnQob3B0aW9ucykKKyAgICAgICAgc2VsZi5hc3NlcnRF cXVhbHMob3B0aW9ucy5jb25maWd1cmF0aW9uLCAnZGVmYXVsdCcpCisgICAgICAgIHNlbGYuYXNz ZXJ0VHJ1ZShwb3J0LmRlZmF1bHRfY29uZmlndXJhdGlvbl9jYWxsZWQpCisKIGlmIF9fbmFtZV9f ID09ICdfX21haW5fXyc6CiAgICAgdW5pdHRlc3QubWFpbigpCg==