54918 2011-02-21 17:34:50 -0800 PrettyPatch.rb doesn't handle diffs where lines matching /^--/ are missing properly 2011-02-21 19:45:06 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 dpranke webkit-unassigned aroben ojan oldest_to_newest 354928 0 dpranke 2011-02-21 17:34:50 -0800 LayoutTests/platform/mac/fast/objc/dom-html-select-activate.html has a few statements of the form log("-- foo") in it, which causes a text line containing two dashes at the start of the line to be output in dumpAsText() mode. If the test fails in such a way that those lines aren't output (as it does on chromium-mac at the moment), then the unified diff contains a line with "---" at the start. PrettyPatch.rb interprets that as the start of a new file for diffing, and gets confused when there's no filename, and errors out. I think, in FileDiff.parse, it needs to be taught to understand the "@@ -1,26 +1,6 @@" line and skip over the appropriate number of lines of input correctly, rather than just looking for a line containing "---". My Ruby is weak, and I am lazy, and PrettyPatch appears to have no unit tests, so I haven't actually submitted a patch to do this. If, however, you change dom-html-select-activate to the contents of the first attachment, you'll get the diff in the second attachment, and old-run-webkit-tests will keel over with PrettyPatch failing. In the meantime, we could also just change the test and the expected output to not contain lines starting with "--" :). I'll attach a patch to do that as well. 354929 1 83244 dpranke 2011-02-21 17:37:06 -0800 Created attachment 83244 sample changed version of the test file that will cause PrettyPatch to crash 354930 2 83245 dpranke 2011-02-21 17:37:33 -0800 Created attachment 83245 the diff produced by the test in the first attachment. PrettyPatch will crash on this directly. 354936 3 83247 dpranke 2011-02-21 17:45:19 -0800 Created attachment 83247 Patch 354995 4 83247 ojan 2011-02-21 19:39:47 -0800 Comment on attachment 83247 Patch I'd eventually like to see prettypatch fixed. Can you file a separate bug for that and make this bug specifically about the layout test? 354997 5 dpranke 2011-02-21 19:44:13 -0800 (In reply to comment #4) > (From update of attachment 83247 [details]) > I'd eventually like to see prettypatch fixed. Can you file a separate bug for that and make this bug specifically about the layout test? Sure. 354998 6 dpranke 2011-02-21 19:45:06 -0800 Committed r79281: <http://trac.webkit.org/changeset/79281> 83244 2011-02-21 17:37:06 -0800 2011-02-21 17:37:06 -0800 sample changed version of the test file that will cause PrettyPatch to crash dom-html-select-activate.html text/html 412 dpranke PCFET0NUWVBFIGh0bWw+CjxodG1sPgo8aGVhZD48dGl0bGU+PC90aXRsZT48L2hlYWQ+Cjxib2R5 Pgo8cHJlIGlkPSJjb25zb2xlIj48L3ByZT4KPHNjcmlwdD4KICAgIGZ1bmN0aW9uIGxvZyhtc2cp IHsKICAgICAgICBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgiY29uc29sZSIpLmFwcGVuZENoaWxk KGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKG1zZyArICJcbiIpKTsKICAgIH0KCiAgICBpZiAoIXdp bmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikgewogICAgICAgbG9nKCJUaGlzIHRlc3QgY2FuIG9u bHkgYmUgcnVuIHVuZGVyIER1bXBSZW5kZXJUcmVlLiIpOwogICAgfQogICAgbGF5b3V0VGVzdENv bnRyb2xsZXIuZHVtcEFzVGV4dCgpOwogICAgbG9nKCJGQUlMIik7CiAgICA8L3NjcmlwdD4KPC9i b2R5Pgo8L2h0bWw+Cg== 83245 2011-02-21 17:37:33 -0800 2011-02-21 17:37:33 -0800 the diff produced by the test in the first attachment. PrettyPatch will crash on this directly. dom-html-select-activate-diffs.txt text/plain 961 dpranke LS0tIC90bXAvbGF5b3V0LXRlc3QtcmVzdWx0cy9wbGF0Zm9ybS9tYWMvZmFzdC9vYmpjL2RvbS1o dG1sLXNlbGVjdC1hY3RpdmF0ZS1leHBlY3RlZC50eHQJMjAxMS0wMi0yMSAxNzoyNzowNC4wMDAw MDAwMDAgLTA4MDAKKysrIC90bXAvbGF5b3V0LXRlc3QtcmVzdWx0cy9wbGF0Zm9ybS9tYWMvZmFz dC9vYmpjL2RvbS1odG1sLXNlbGVjdC1hY3RpdmF0ZS1hY3R1YWwudHh0CTIwMTEtMDItMjEgMTc6 Mjc6MDQuMDAwMDAwMDAwIC0wODAwCkBAIC0xLDIxICsxLDIgQEAKLSAgICAgIAotVGhpcyB0ZXN0 cyB0aGUgUHJpdmF0ZSBPYmplY3RpdmUtQyBET00gSFRNTCBCaW5kaW5ncyBBUEkgZm9yIHNlbGVj dGluZyBpbmRleGVzIG9mIGEgPHNlbGVjdD4gdGhhdCBhbHNvIGFjdGl2YXRlIHRoZSAiY2hhbmdl IiBldmVudC4KLQotU2VsZWN0IGEgc2luZ2xlIGluZGV4IGluIGEgbm9ybWFsIDxzZWxlY3Q+Lgot LS0gY2hhbmdlIGV2ZW50IGZpcmVkIC0tCi1QQVNTIC0gZXhwZWN0ZWQgaW5kZXggd2FzIHNlbGVj dGVkLgotCi1TZWxlY3QgYSBzaW5nbGUgaW5kZXggaW4gYSBzaXplZCA8c2VsZWN0Pi4KLS0tIGNo YW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAtIGV4cGVjdGVkIGluZGV4IHdhcyBzZWxlY3RlZC4K LQotU2VsZWN0IG11bHRpcGxlIGluZGV4ZXMsIGluIGEgPHNlbGVjdCBtdWx0aXBsZT4uCi0tLSBj aGFuZ2UgZXZlbnQgZmlyZWQgLS0KLS0tIGNoYW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAtIHNl bGVjdGVkIG9wdGlvbnMgd2VyZSB3aGF0IHdlIGV4cGVjdGVkLgotCi1TZWxlY3QgbXVsdGlwbGUg aW5kZXhlcywgd2l0aCBtdWx0aS1zZWxlY3Qgb24sIGluIGEgPHNlbGVjdCBtdWx0aXBsZT4uCi0t LSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KLS0tIGNoYW5nZSBldmVudCBmaXJlZCAtLQotUEFTUyAt IHNlbGVjdGVkIG9wdGlvbnMgd2VyZSB3aGF0IHdlIGV4cGVjdGVkLgorRkFJTAogCg== 83247 2011-02-21 17:45:19 -0800 2011-02-21 19:39:46 -0800 Patch bug-54918-20110221174518.patch text/plain 2741 dpranke U3VidmVyc2lvbiBSZXZpc2lvbjogNzkyNjEKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCBlNDAzNThmZDc1NGE5YjU2NjIyM2Zl NWNmYzE1MTQ1NDhiOWYwZWI1Li4zY2JlOThlZWFiM2RhMzA5NzhiZmEzMWYwNmEyYWEzZmE1M2Rk Y2YyIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDItMjEgIERpcmsgUHJhbmtlICA8ZHBy YW5rZUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgVGhpcyBwYXRjaCBjaGFuZ2VzIGEgdGVzdCB0byB3b3JrIGFyb3VuZCBhIGJ1 ZyBpbiBQcmV0dHlQYXRjaCB3aGVyZQorICAgICAgICBpdCB3aWxsIGNyYXNoIGlmIGxpbmVzIHN0 YXJ0aW5nIHdpdGggIi0tIiBhcmUgbWlzc2luZyBmcm9tIHRoZSBhY3R1YWwKKyAgICAgICAgb3V0 cHV0LiBUaGUgYWN0dWFsIGZpeCBuZWVkZWQgdG8gUHJldHR5UGF0Y2ggaXMgZGVzY3JpYmVkIGlu IHRoZQorICAgICAgICBidWcuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTU0OTE4CisKKyAgICAgICAgKiBwbGF0Zm9ybS9tYWMvZmFzdC9vYmpjL2Rv bS1odG1sLXNlbGVjdC1hY3RpdmF0ZS1leHBlY3RlZC50eHQ6CisgICAgICAgICogcGxhdGZvcm0v bWFjL2Zhc3Qvb2JqYy9kb20taHRtbC1zZWxlY3QtYWN0aXZhdGUuaHRtbDoKKwogMjAxMS0wMi0y MSAgQW1pIEZpc2NobWFuICA8ZmlzY2htYW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmll d2VkIGJ5IEVyaWMgQ2FybHNvbi4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3BsYXRmb3JtL21h Yy9mYXN0L29iamMvZG9tLWh0bWwtc2VsZWN0LWFjdGl2YXRlLWV4cGVjdGVkLnR4dCBiL0xheW91 dFRlc3RzL3BsYXRmb3JtL21hYy9mYXN0L29iamMvZG9tLWh0bWwtc2VsZWN0LWFjdGl2YXRlLWV4 cGVjdGVkLnR4dAppbmRleCBhYzI3NjY2OTQ1NWNjN2NmODI4NmQ4OTUzMjk2ZTE1ZTAzZjQzMWU0 Li5kNWFlMDFiODlmYWJmMjU3NGE3ZmYwN2Y5ZDMxMmM4YTc5MTI3Y2NhIDEwMDY0NAotLS0gYS9M YXlvdXRUZXN0cy9wbGF0Zm9ybS9tYWMvZmFzdC9vYmpjL2RvbS1odG1sLXNlbGVjdC1hY3RpdmF0 ZS1leHBlY3RlZC50eHQKKysrIGIvTGF5b3V0VGVzdHMvcGxhdGZvcm0vbWFjL2Zhc3Qvb2JqYy9k b20taHRtbC1zZWxlY3QtYWN0aXZhdGUtZXhwZWN0ZWQudHh0CkBAIC0yLDIwICsyLDIwIEBACiBU aGlzIHRlc3RzIHRoZSBQcml2YXRlIE9iamVjdGl2ZS1DIERPTSBIVE1MIEJpbmRpbmdzIEFQSSBm b3Igc2VsZWN0aW5nIGluZGV4ZXMgb2YgYSA8c2VsZWN0PiB0aGF0IGFsc28gYWN0aXZhdGUgdGhl ICJjaGFuZ2UiIGV2ZW50LgogCiBTZWxlY3QgYSBzaW5nbGUgaW5kZXggaW4gYSBub3JtYWwgPHNl bGVjdD4uCi0tLSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KKyoqIGNoYW5nZSBldmVudCBmaXJlZCAq KgogUEFTUyAtIGV4cGVjdGVkIGluZGV4IHdhcyBzZWxlY3RlZC4KIAogU2VsZWN0IGEgc2luZ2xl IGluZGV4IGluIGEgc2l6ZWQgPHNlbGVjdD4uCi0tLSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KKyoq IGNoYW5nZSBldmVudCBmaXJlZCAqKgogUEFTUyAtIGV4cGVjdGVkIGluZGV4IHdhcyBzZWxlY3Rl ZC4KIAogU2VsZWN0IG11bHRpcGxlIGluZGV4ZXMsIGluIGEgPHNlbGVjdCBtdWx0aXBsZT4uCi0t LSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KLS0tIGNoYW5nZSBldmVudCBmaXJlZCAtLQorKiogY2hh bmdlIGV2ZW50IGZpcmVkICoqCisqKiBjaGFuZ2UgZXZlbnQgZmlyZWQgKioKIFBBU1MgLSBzZWxl Y3RlZCBvcHRpb25zIHdlcmUgd2hhdCB3ZSBleHBlY3RlZC4KIAogU2VsZWN0IG11bHRpcGxlIGlu ZGV4ZXMsIHdpdGggbXVsdGktc2VsZWN0IG9uLCBpbiBhIDxzZWxlY3QgbXVsdGlwbGU+LgotLS0g Y2hhbmdlIGV2ZW50IGZpcmVkIC0tCi0tLSBjaGFuZ2UgZXZlbnQgZmlyZWQgLS0KKyoqIGNoYW5n ZSBldmVudCBmaXJlZCAqKgorKiogY2hhbmdlIGV2ZW50IGZpcmVkICoqCiBQQVNTIC0gc2VsZWN0 ZWQgb3B0aW9ucyB3ZXJlIHdoYXQgd2UgZXhwZWN0ZWQuCiAKZGlmZiAtLWdpdCBhL0xheW91dFRl c3RzL3BsYXRmb3JtL21hYy9mYXN0L29iamMvZG9tLWh0bWwtc2VsZWN0LWFjdGl2YXRlLmh0bWwg Yi9MYXlvdXRUZXN0cy9wbGF0Zm9ybS9tYWMvZmFzdC9vYmpjL2RvbS1odG1sLXNlbGVjdC1hY3Rp dmF0ZS5odG1sCmluZGV4IDA1ODU4YWU1OWNkNjkyZmI1NDI3NWU4MTMyYmJmY2NkZDViMDI5NGUu LjZkZTBjZWZhZjg2NTcxMjIxMDVmMzZkMGM2ZGEwMDFhNjdjMDJlNGIgMTAwNjQ0Ci0tLSBhL0xh eW91dFRlc3RzL3BsYXRmb3JtL21hYy9mYXN0L29iamMvZG9tLWh0bWwtc2VsZWN0LWFjdGl2YXRl Lmh0bWwKKysrIGIvTGF5b3V0VGVzdHMvcGxhdGZvcm0vbWFjL2Zhc3Qvb2JqYy9kb20taHRtbC1z ZWxlY3QtYWN0aXZhdGUuaHRtbApAQCAtNDAsNyArNDAsNyBAQAogICAgIH0KIAogICAgIGZ1bmN0 aW9uIG9uQ2hhbmdlRXZlbnRIYW5kbGVyKGV2ZW50KSB7Ci0gICAgICAgIGxvZygiLS0gY2hhbmdl IGV2ZW50IGZpcmVkIC0tIik7CisgICAgICAgIGxvZygiKiogY2hhbmdlIGV2ZW50IGZpcmVkICoq Iik7CiAgICAgfQogCiAgICAgZnVuY3Rpb24gYXNzZXJ0U2VsZWN0ZWRJbmRleChlbGVtLCBpbmRl eCkgewo=