Summary: | Fix the commit-log-editor after r167243 and add more unit tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||||
Component: | Tools / Tests | Assignee: | Csaba Osztrogonác <ossy> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, commit-queue, glenn, jberta.u-szeged, joepeck, ossy | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Csaba Osztrogonác
2014-04-16 03:12:07 PDT
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 |