http://trac.webkit.org/changeset/167243 broke a webkitpy unit test and update landed in http://trac.webkit.org/r167296 Sorry for causing troubles, I didn't know if there is a webkitpy unit test for the Tools/Scripts/commit-log-editor perl script. I checked the webkitperl directory and haven't seen any unit test for this script, that's why I didn't want to expect the author to implement a unit test from the scratch for a script which can't be mockable and relies on filesystem changes. But know we know that we have an existing unit test for it in webkitpy, so we should add unit tests for r167243. Additionally I think the new behaviour which caused the unit test failure isn't the expected result, we didn't want to remove the extra newline after directory names. I think it is a minor bug which can be fixed easily.
I think I got the problem. Check this small changelog entry: ------------------------------------------------------------------------------ 2014-04-14 Peter Molnar <pmolnar.u-szeged@partner.samsung.com> Fix incorrect indentations in CodeGeneratorJS.pm introduced in r165521 https://bugs.webkit.org/show_bug.cgi?id=131613 Reviewed by Csaba Osztrogonác. * bindings/scripts/CodeGeneratorJS.pm: (GenerateImplementation): Fixed 5-space indentation. * bindings/scripts/test/JS/JSTestNondeterministic.cpp: Updated the tests accordingly. ------------------------------------------------------------------------------ In this case the common prefix was: ------------------------------------------------------------------------------ 2014-04-14 Peter Molnar <pmolnar.u-szeged@partner.samsung.com> Fix incorrect indentations in CodeGeneratorJS.pm introduced in r165521 https://bugs.webkit.org/show_bug.cgi?id=131613 Reviewed by Csaba Osztrogonác. ------------------------------------------------------------------------------ And the non-common part was: ------------------------------------------------------------------------------ * bindings/scripts/CodeGeneratorJS.pm: (GenerateImplementation): Fixed 5-space indentation. * bindings/scripts/test/JS/JSTestNondeterministic.cpp: Updated the tests accordingly. ------------------------------------------------------------------------------ The problem was that commit-log-editor relied on the extra \n\n in non-common parts and it didn't add any \n after the name of the label, eg: "WebCore:"
Created attachment 229436 [details] Patch WIP patch. It works for the mentioned case, but I haven't tested it on other cases. Additionally I'm not sure if it is the best fix, because the non-common part still starts with a \n. We really need more unit tests for it. Maybe we should rewrite commit-log-editor in python and make it mockable not to depend on local file system changes.
I won't be online until next tuesday, I can fix it properly after that. If somebody can play with my WIP patch, feel free to pick it up.
Created attachment 235692 [details] Patch
Attachment 235692 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:202: [CommitMessageForThisCommitTest.test_commit_message_for_unreviewed_changelogs_with_different_messages] Passing unexpected keyword argument 'return_stderr' in function call [pylint/E1123] [5] ERROR: Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:223: [CommitMessageForThisCommitTest.test_commit_message_for_one_reviewed_changelog] Passing unexpected keyword argument 'return_stderr' in function call [pylint/E1123] [5] ERROR: Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:277: [CommitMessageForThisCommitTest.test_commit_message_for_changelogs_with_different_messages] Passing unexpected keyword argument 'return_stderr' in function call [pylint/E1123] [5] ERROR: Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:304: [CommitMessageForThisCommitTest.test_commit_message_for_one_rollout_changelog] Passing unexpected keyword argument 'return_stderr' in function call [pylint/E1123] [5] ERROR: Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:330: [CommitMessageForThisCommitTest.test_commit_message_for_rollout_changelogs_with_different_directories] Passing unexpected keyword argument 'return_stderr' in function call [pylint/E1123] [5] Total errors found: 5 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 235811 [details] Patch
Comment on attachment 235811 [details] Patch Clearing flags on attachment: 235811 Committed r172029: <http://trac.webkit.org/changeset/172029>
All reviewed patches have been landed. Closing bug.
This change caused a regression for me: <https://webkit.org/b/135744> commit-log-message has extra blank line at the top of pre-populated message