30645 2009-10-21 15:25:42 -0700 REGRESSION: 2 failures in run-webkit-unittests 2009-10-26 13:54:04 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 eric webkit-unassigned carol commit-queue levin oldest_to_newest 156661 0 eric 2009-10-21 15:25:42 -0700 ====================================================================== FAIL: test_braces (modules.cpp_style_unittest.WebKitStyleTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/eseidel/Projects/WebKit/WebKitTools/Scripts/modules/cpp_style_unittest.py", line 3309, in test_braces 'Code inside a namespace should not be indented. [whitespace/indent] [4]') File "/Users/eseidel/Projects/WebKit/WebKitTools/Scripts/modules/cpp_style_unittest.py", line 219, in assert_multi_line_lint self.assertEquals(expected_message, self.perform_multi_line_lint(code, file_name)) AssertionError: 'Code inside a namespace should not be indented. [whitespace/indent] [4]' != '' ====================================================================== FAIL: test_indentation (modules.cpp_style_unittest.WebKitStyleTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/eseidel/Projects/WebKit/WebKitTools/Scripts/modules/cpp_style_unittest.py", line 2823, in test_indentation 'foo.h') File "/Users/eseidel/Projects/WebKit/WebKitTools/Scripts/modules/cpp_style_unittest.py", line 219, in assert_multi_line_lint self.assertEquals(expected_message, self.perform_multi_line_lint(code, file_name)) AssertionError: 'Code inside a namespace should not be indented. [whitespace/indent] [4]' != '' ---------------------------------------------------------------------- Ran 166 tests in 135.229s FAILED (failures=2) Probably caused from http://trac.webkit.org/changeset/49690 156680 1 41614 carol 2009-10-21 16:01:23 -0700 Created attachment 41614 Proposed Patch Added back some spaces that were missing from test cases and fixed the expected result in one other. I do not know how these test cases got in there. 156682 2 eric 2009-10-21 16:06:44 -0700 I thought that the new style was not to indent code inside namespaces? 156685 3 41614 darin 2009-10-21 16:07:21 -0700 Comment on attachment 41614 Proposed Patch > self.assert_multi_line_lint( > 'namespace WebCore {\n' > '#if 0\n' > - 'class Document {\n' > + ' class Document {\n' > '};\n' Really? Indent the line starting the class definition but not the line ending it? 156766 4 carol 2009-10-21 20:34:43 -0700 (In reply to comment #2) > I thought that the new style was not to indent code inside namespaces? Yes, you are right. And that is what the cpp_style verifies. 156771 5 carol 2009-10-21 20:40:36 -0700 (In reply to comment #3) > (From update of attachment 41614 [details]) > > self.assert_multi_line_lint( > > 'namespace WebCore {\n' > > '#if 0\n' > > - 'class Document {\n' > > + ' class Document {\n' > > '};\n' > > Really? Indent the line starting the class definition but not the line ending > it? This code has bad style, but currently we do not have a style checker that is able to check indentation other than for the following cases: 1. First level inside a namespace (should be 0). 2. Indentation should be a multiple of 4 columns. The purpose of the test above was to verify that we do not complain about the indentation of the inner class. 156777 6 carol 2009-10-21 20:46:39 -0700 (In reply to comment #3) > (From update of attachment 41614 [details]) > > self.assert_multi_line_lint( > > 'namespace WebCore {\n' > > '#if 0\n' > > - 'class Document {\n' > > + ' class Document {\n' > > '};\n' > > Really? Indent the line starting the class definition but not the line ending > it? I take my previous comment back. The correct style as I read the style document is not to indent the class definition. This is bad style and the checker should complain. I agree that this patch is hard to review because it does not give enough context. The next line in the file says that the checker should complain about the indented class definition. 157902 7 41614 commit-queue 2009-10-26 13:53:57 -0700 Comment on attachment 41614 Proposed Patch Clearing flags on attachment: 41614 Committed r50091: <http://trac.webkit.org/changeset/50091> 157904 8 commit-queue 2009-10-26 13:54:04 -0700 All reviewed patches have been landed. Closing bug. 41614 2009-10-21 16:01:23 -0700 2009-10-26 13:53:56 -0700 Proposed Patch patchFor30645-1.patch text/plain 2153 carol SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDQ5OTI0KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMTAtMjEgIENhcm9sIFN6YWJvICA8Y2Fy b2wuc3phYm9Abm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFJFR1JFU1NJT046IDIgZmFpbHVyZXMgaW4gcnVuLXdlYmtpdC11bml0dGVz dHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMwNjQ1 CisKKyAgICAgICAgKiBTY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlX3VuaXR0ZXN0LnB5OgorICAg ICAgICBGaXhlZCBhIGZldyB0ZXN0IHNjZW5hcmlvcyB3aGljaCBhcHBhcmVudGx5IGxvc3Qgc29t ZSBzcGFjZXMgZnJvbSAKKyAgICAgICAgdGV4dCBsaXRlcmFscy4KKwogMjAwOS0xMC0yMSAgQWxl amFuZHJvIEcuIENhc3RybyAgPGFsZXhAaWdhbGlhLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBi eSBHdXN0YXZvIE5vcm9uaGEuCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3Bw X3N0eWxlX3VuaXR0ZXN0LnB5Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYktpdFRvb2xzL1NjcmlwdHMvbW9k dWxlcy9jcHBfc3R5bGVfdW5pdHRlc3QucHkJKHJldmlzaW9uIDQ5OTI0KQorKysgV2ViS2l0VG9v bHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91bml0dGVzdC5weQkod29ya2luZyBjb3B5KQpA QCAtMjgxOSwxMiArMjgxOSwxMiBAQCBjbGFzcyBXZWJLaXRTdHlsZVRlc3QoQ3BwU3R5bGVUZXN0 QmFzZSk6CiAgICAgICAgICAgICAnICAgIGludCBteVZhcmlhYmxlO1xuJwogICAgICAgICAgICAg J307XG4nCiAgICAgICAgICAgICAnfScsCi0gICAgICAgICAgICAnQ29kZSBpbnNpZGUgYSBuYW1l c3BhY2Ugc2hvdWxkIG5vdCBiZSBpbmRlbnRlZC4gIFt3aGl0ZXNwYWNlL2luZGVudF0gWzRdJywK KyAgICAgICAgICAgICcnLAogICAgICAgICAgICAgJ2Zvby5oJykKICAgICAgICAgc2VsZi5hc3Nl cnRfbXVsdGlfbGluZV9saW50KAogICAgICAgICAgICAgJ25hbWVzcGFjZSBPdXRlck5hbWVzcGFj ZSB7XG4nCi0gICAgICAgICAgICAnbmFtZXNwYWNlIElubmVyTmFtZXNwYWNlIHtcbicKLSAgICAg ICAgICAgICdjbGFzcyBEb2N1bWVudCB7XG4nCisgICAgICAgICAgICAnICAgIG5hbWVzcGFjZSBJ bm5lck5hbWVzcGFjZSB7XG4nCisgICAgICAgICAgICAnICAgIGNsYXNzIERvY3VtZW50IHtcbicK ICAgICAgICAgICAgICd9O1xuJwogICAgICAgICAgICAgJ307XG4nCiAgICAgICAgICAgICAnfScs CkBAIC0yODMzLDcgKzI4MzMsNyBAQCBjbGFzcyBXZWJLaXRTdHlsZVRlc3QoQ3BwU3R5bGVUZXN0 QmFzZSk6CiAgICAgICAgIHNlbGYuYXNzZXJ0X211bHRpX2xpbmVfbGludCgKICAgICAgICAgICAg ICduYW1lc3BhY2UgV2ViQ29yZSB7XG4nCiAgICAgICAgICAgICAnI2lmIDBcbicKLSAgICAgICAg ICAgICdjbGFzcyBEb2N1bWVudCB7XG4nCisgICAgICAgICAgICAnICAgIGNsYXNzIERvY3VtZW50 IHtcbicKICAgICAgICAgICAgICd9O1xuJwogICAgICAgICAgICAgJyNlbmRpZlxuJwogICAgICAg ICAgICAgJ30nLApAQCAtMzMwNiw3ICszMzA2LDcgQEAgY2xhc3MgV2ViS2l0U3R5bGVUZXN0KENw cFN0eWxlVGVzdEJhc2UpOgogICAgICAgICAgICAgJ25hbWVzcGFjZSBXZWJDb3JlIHtcbicKICAg ICAgICAgICAgICdpbnQgZm9vO1xuJwogICAgICAgICAgICAgJ307XG4nLAotICAgICAgICAgICAg J0NvZGUgaW5zaWRlIGEgbmFtZXNwYWNlIHNob3VsZCBub3QgYmUgaW5kZW50ZWQuICBbd2hpdGVz cGFjZS9pbmRlbnRdIFs0XScpCisgICAgICAgICAgICAnJykKICAgICAgICAgc2VsZi5hc3NlcnRf bXVsdGlfbGluZV9saW50KAogICAgICAgICAgICAgJ2ZvciAoaW50IGkgPSAwOyBpIDwgMTA7IGkr Kykge1xuJwogICAgICAgICAgICAgJyAgICBEb1NvbWV0aGluZygpO1xuJwo=