96149 2012-09-07 14:17:42 -0700 Handle non-existant TestExpectations files gracefully 2012-09-10 16:10:35 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ojan ojan abarth dpranke eric tony webkit.review.bot oldest_to_newest 714973 0 ojan 2012-09-07 14:17:42 -0700 Handle non-existant TestExpectations files gracefully 714976 1 162868 ojan 2012-09-07 14:18:54 -0700 Created attachment 162868 Patch 714988 2 eric 2012-09-07 14:30:55 -0700 I'm not sure what this is for? 714994 3 ojan 2012-09-07 14:35:04 -0700 Right now, running "run-webkit-tests --lint-test-files" throws a stack halfway through because LayoutTests/platform/win-7sp0/TestExpectations doesn't exist. I'd rather make this code change than create a dummy file, especially as we don't use nrwt on Apple Windows still, e.g. see LayoutTests/platform/win/TestExpectations, which has a comment at the top saying the file is unused, but people still add lines to it. 714995 4 ojan 2012-09-07 14:36:51 -0700 I suppose the plan is to get rid of Skipped files and just have TestExpectations in the end, but I still don't think we need to require one such file for each subdirectory. 714997 5 162868 eric 2012-09-07 14:41:03 -0700 Comment on attachment 162868 Patch OK. 715031 6 ojan 2012-09-07 15:06:07 -0700 Committed r127925: <http://trac.webkit.org/changeset/127925> 716564 7 dpranke 2012-09-10 15:04:29 -0700 I'm not a huge fan of this solution; it seems like an error to me to say that you are using a TestExpectations file and then not have it actually exist. I think we should either modify expectatation_files() to not return files that don't exist, or just create a stub file (like I've done on the other ports); the latter seems preferable. 716571 8 tony 2012-09-10 15:10:11 -0700 (In reply to comment #7) > I'm not a huge fan of this solution; it seems like an error to me to say that you are using a TestExpectations file and then not have it actually exist. I think we should either modify expectatation_files() to not return files that don't exist, or just create a stub file (like I've done on the other ports); the latter seems preferable. FWIW, I suggested this because the stub file doesn't prevent people from adding lines. See http://trac.webkit.org/changeset/127927#file7 . In practice, you would only accidentally not have a TestExpectations file when creating a new port. I think you would end up needing to create the file immediately, so I'm not sure there's any fear about someone forgetting to create TestExpectations. 716597 9 dpranke 2012-09-10 15:24:00 -0700 (In reply to comment #8) > (In reply to comment #7) > > I'm not a huge fan of this solution; it seems like an error to me to say that you are using a TestExpectations file and then not have it actually exist. I think we should either modify expectatation_files() to not return files that don't exist, or just create a stub file (like I've done on the other ports); the latter seems preferable. > > FWIW, I suggested this because the stub file doesn't prevent people from adding lines. See http://trac.webkit.org/changeset/127927#file7 . > This is true. I'm not sure that this is a huge problem in practice, but then, I'm not sure how big an issue missing files are either (assuming code returning the list of files that are actually expected to exist). To me, the biggest problem is that this patch seems like a hack - one routine is returning something it shouldn't and we're working around it. We should fix the real problem. > In practice, you would only accidentally not have a TestExpectations file when creating a new port. I think you would end up needing to create the file immediately, so I'm not sure there's any fear about someone forgetting to create TestExpectations. I'm not sure what you're getting at here? Are you agreeing with me that missing files should be treated as errors, or saying something else? 716636 10 tony 2012-09-10 16:10:35 -0700 (In reply to comment #9) > I'm not sure what you're getting at here? Are you agreeing with me that missing files should be treated as errors, or saying something else? I'm saying that missing files should not be treated as errors. Modifying expectatation_files() to not return files that don't exist is also fine with me. 162868 2012-09-07 14:18:54 -0700 2012-09-07 14:41:03 -0700 Patch bug-96149-20120907211833.patch text/plain 2907 ojan U3VidmVyc2lvbiBSZXZpc2lvbjogMTI3OTE3CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNjk3MDVkM2U4MWY5NWIzMTE5NDE5OWYyNzJiODNjOTJk YzUxYTJjYy4uZGRkNDU3MDZhMTdmMzczMGU1MTQ4MGIxODQ5NzI1YjdhNGMyZTVjZCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5 IEBACisyMDEyLTA5LTA3ICBPamFuIFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CisKKyAgICAg ICAgSGFuZGxlIG5vbi1leGlzdGFudCBUZXN0RXhwZWN0YXRpb25zIGZpbGVzIGdyYWNlZnVsbHkK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTk2MTQ5CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRXZlbiB3aGVu IHdlIGdldCByaWQgb2YgU2tpcHBlZCBmaWxlcywgd2Ugc2hvdWxkbid0IGhhdmUgZWFjaCBwb3J0 CisgICAgICAgIHJlcXVpcmUgYSBUZXN0RXhwZWN0YXRpb25zIGZpbGUgZm9yIGVhY2ggcGxhdGZv cm0gaWYgaXQgZG9lc24ndAorICAgICAgICBuZWVkIG9uZS4KKworICAgICAgICAqIFNjcmlwdHMv d2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZS5weToKKyAgICAgICAgKFBvcnQuZXhwZWN0 YXRpb25zX2RpY3QpOgorICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3Bv cnQvYmFzZV91bml0dGVzdC5weToKKyAgICAgICAgKFBvcnRUZXN0LnRlc3Rfbm9uZXhpc3RhbnRf ZXhwZWN0YXRpb25zKToKKwogMjAxMi0wOS0wNyAgVG9ueSBDaGFuZyAgPHRvbnlAY2hyb21pdW0u b3JnPgogCiAgICAgICAgIFJlZ3Jlc3Npb246IGNoZWNrLXdlYmtpdC1zdHlsZSBubyBsb25nZXIg Y2hlY2tzIFRlc3RFeHBlY3RhdGlvbnMgZm9yIHN5bnRheCBlcnJvcnMKZGlmZiAtLWdpdCBhL1Rv b2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZS5weSBiL1Rvb2xzL1Nj cmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL3BvcnQvYmFzZS5weQppbmRleCBhNGY2YTU0ZDhk MzU3NWI5YzUyYjI4NzQ5NmVkZGEwMTJlNGIwYTI1Li44Njg2NDM3MTE1YWY5ZDQyOGQ1Y2QxYzFm YjRiOWNhMDZlODc1MjMzIDEwMDc1NQotLS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91 dF90ZXN0cy9wb3J0L2Jhc2UucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRf dGVzdHMvcG9ydC9iYXNlLnB5CkBAIC0xMDA2LDcgKzEwMDYsOCBAQCBjbGFzcyBQb3J0KG9iamVj dCk6CiAgICAgICAgIGV4cGVjdGF0aW9ucyA9IE9yZGVyZWREaWN0KCkKIAogICAgICAgICBmb3Ig cGF0aCBpbiBzZWxmLmV4cGVjdGF0aW9uc19maWxlcygpOgotICAgICAgICAgICAgZXhwZWN0YXRp b25zW3BhdGhdID0gc2VsZi5fZmlsZXN5c3RlbS5yZWFkX3RleHRfZmlsZShwYXRoKQorICAgICAg ICAgICAgaWYgc2VsZi5fZmlsZXN5c3RlbS5leGlzdHMocGF0aCk6CisgICAgICAgICAgICAgICAg ZXhwZWN0YXRpb25zW3BhdGhdID0gc2VsZi5fZmlsZXN5c3RlbS5yZWFkX3RleHRfZmlsZShwYXRo KQogCiAgICAgICAgIGZvciBwYXRoIGluIHNlbGYuZ2V0X29wdGlvbignYWRkaXRpb25hbF9leHBl Y3RhdGlvbnMnLCBbXSk6CiAgICAgICAgICAgICBleHBhbmRlZF9wYXRoID0gc2VsZi5fZmlsZXN5 c3RlbS5leHBhbmR1c2VyKHBhdGgpCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5 L2xheW91dF90ZXN0cy9wb3J0L2Jhc2VfdW5pdHRlc3QucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L2xheW91dF90ZXN0cy9wb3J0L2Jhc2VfdW5pdHRlc3QucHkKaW5kZXggYzY4ZTQ0MWFhY2Qw ZDRiNjYwMjJmOWU4ZTljYzZjMjliMDUyYjQ2MC4uZjZhNmViMDFmNjM5OGMxZmY2YTg4MTRjM2Y2 OTIyMDIwZDU1NGY0ZiAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRf dGVzdHMvcG9ydC9iYXNlX3VuaXR0ZXN0LnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkv bGF5b3V0X3Rlc3RzL3BvcnQvYmFzZV91bml0dGVzdC5weQpAQCAtMjY0LDYgKzI2NCwxMiBAQCBj bGFzcyBQb3J0VGVzdCh1bml0dGVzdC5UZXN0Q2FzZSk6CiAgICAgICAgICAgICBbKCcvdG1wL2xv Y2FsLWJhc2VsaW5lcycsICdmYXN0L3Rlc3QtZXhwZWN0ZWQudHh0JyldKQogICAgICAgICBzZWxm LmFzc2VydEVxdWFsKHBvcnQuYmFzZWxpbmVfcGF0aCgpLCAnL2ZvbycpCiAKKyAgICBkZWYgdGVz dF9ub25leGlzdGFudF9leHBlY3RhdGlvbnMoc2VsZik6CisgICAgICAgIHBvcnQgPSBzZWxmLm1h a2VfcG9ydChwb3J0X25hbWU9J2ZvbycpCisgICAgICAgIHBvcnQuZXhwZWN0YXRpb25zX2ZpbGVz ID0gbGFtYmRhOiBbJy9tb2NrLWNoZWNrb3V0L0xheW91dFRlc3RzL3BsYXRmb3JtL2V4aXN0cy9U ZXN0RXhwZWN0YXRpb25zJywgJy9tb2NrLWNoZWNrb3V0L0xheW91dFRlc3RzL3BsYXRmb3JtL25v bmV4aXN0YW50L1Rlc3RFeHBlY3RhdGlvbnMnXQorICAgICAgICBwb3J0Ll9maWxlc3lzdGVtLndy aXRlX3RleHRfZmlsZSgnL21vY2stY2hlY2tvdXQvTGF5b3V0VGVzdHMvcGxhdGZvcm0vZXhpc3Rz L1Rlc3RFeHBlY3RhdGlvbnMnLCAnJykKKyAgICAgICAgc2VsZi5hc3NlcnRFcXVhbHMoJ1xuJy5q b2luKHBvcnQuZXhwZWN0YXRpb25zX2RpY3QoKS5rZXlzKCkpLCAnL21vY2stY2hlY2tvdXQvTGF5 b3V0VGVzdHMvcGxhdGZvcm0vZXhpc3RzL1Rlc3RFeHBlY3RhdGlvbnMnKQorCiAgICAgZGVmIHRl c3RfYWRkaXRpb25hbF9leHBlY3RhdGlvbnMoc2VsZik6CiAgICAgICAgIHBvcnQgPSBzZWxmLm1h a2VfcG9ydChwb3J0X25hbWU9J2ZvbycpCiAgICAgICAgIHBvcnQucG9ydF9uYW1lID0gJ2ZvbycK