diff options
Diffstat (limited to 'WebKitTools/Scripts/modules')
| -rw-r--r-- | WebKitTools/Scripts/modules/bugzilla.py | 26 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/buildbot.py | 14 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/buildbot_unittest.py | 16 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/committers.py | 31 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/cpp_style.py | 85 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/cpp_style_unittest.py | 100 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/scm.py | 16 | ||||
| -rw-r--r-- | WebKitTools/Scripts/modules/scm_unittest.py | 59 |
8 files changed, 230 insertions, 117 deletions
diff --git a/WebKitTools/Scripts/modules/bugzilla.py b/WebKitTools/Scripts/modules/bugzilla.py index daf3f19..fe81b48 100644 --- a/WebKitTools/Scripts/modules/bugzilla.py +++ b/WebKitTools/Scripts/modules/bugzilla.py @@ -294,7 +294,18 @@ class Bugzilla: self.authenticated = True - def add_patch_to_bug(self, bug_id, patch_file_object, description, comment_text=None, mark_for_review=False): + def _fill_attachment_form(self, description, patch_file_object, comment_text=None, mark_for_review=False, mark_for_commit_queue=False, bug_id=None): + self.browser['description'] = description + self.browser['ispatch'] = ("1",) + self.browser['flag_type-1'] = ('?',) if mark_for_review else ('X',) + self.browser['flag_type-3'] = ('?',) if mark_for_commit_queue else ('X',) + if bug_id: + patch_name = "bug-%s-%s.patch" % (bug_id, timestamp()) + else: + patch_name ="%s.patch" % timestamp() + self.browser.add_file(patch_file_object, "text/plain", patch_name, 'data') + + def add_patch_to_bug(self, bug_id, patch_file_object, description, comment_text=None, mark_for_review=False, mark_for_commit_queue=False): self.authenticate() log('Adding patch "%s" to bug %s' % (description, bug_id)) @@ -304,13 +315,10 @@ class Bugzilla: self.browser.open("%sattachment.cgi?action=enter&bugid=%s" % (self.bug_server_url, bug_id)) self.browser.select_form(name="entryform") - self.browser['description'] = description - self.browser['ispatch'] = ("1",) + self._fill_attachment_form(description, patch_file_object, mark_for_review=mark_for_review, mark_for_commit_queue=mark_for_commit_queue, bug_id=bug_id) if comment_text: log(comment_text) self.browser['comment'] = comment_text - self.browser['flag_type-1'] = ('?',) if mark_for_review else ('X',) - self.browser.add_file(patch_file_object, "text/plain", "bug-%s-%s.patch" % (bug_id, timestamp())) self.browser.submit() def prompt_for_component(self, components): @@ -334,7 +342,7 @@ class Bugzilla: error_message = "\n" + '\n'.join([" " + line.strip() for line in text_lines if line.strip()]) raise BugzillaError("Bug not created: %s" % error_message) - def create_bug_with_patch(self, bug_title, bug_description, component, patch_file_object, patch_description, cc, mark_for_review=False): + def create_bug_with_patch(self, bug_title, bug_description, component, patch_file_object, patch_description, cc, mark_for_review=False, mark_for_commit_queue=False): self.authenticate() log('Creating bug with patch description "%s"' % patch_description) @@ -355,10 +363,8 @@ class Bugzilla: if bug_description: log(bug_description) self.browser['comment'] = bug_description - self.browser['description'] = patch_description - self.browser['ispatch'] = ("1",) - self.browser['flag_type-1'] = ('?',) if mark_for_review else ('X',) - self.browser.add_file(patch_file_object, "text/plain", "%s.patch" % timestamp(), 'data') + + self._fill_attachment_form(patch_description, patch_file_object, mark_for_review=mark_for_review, mark_for_commit_queue=mark_for_commit_queue) response = self.browser.submit() bug_id = self._check_create_bug_response(response.read()) diff --git a/WebKitTools/Scripts/modules/buildbot.py b/WebKitTools/Scripts/modules/buildbot.py index 4478429..e948d8c 100644 --- a/WebKitTools/Scripts/modules/buildbot.py +++ b/WebKitTools/Scripts/modules/buildbot.py @@ -83,11 +83,19 @@ class BuildBot: builders.append(builder) return builders - def core_builders_are_green(self): + def red_core_builders(self): + red_builders = [] for builder in self._builder_statuses_with_names_matching_regexps(self.builder_statuses(), self.core_builder_names_regexps): if not builder['is_green']: - return False - return True + red_builders.append(builder) + return red_builders + + def red_core_builders_names(self): + red_builders = self.red_core_builders() + return map(lambda builder: builder['name'], red_builders) + + def core_builders_are_green(self): + return not self.red_core_builders() def builder_statuses(self): build_status_url = self.buildbot_server_url + 'one_box_per_builder' diff --git a/WebKitTools/Scripts/modules/buildbot_unittest.py b/WebKitTools/Scripts/modules/buildbot_unittest.py index 461e5a2..a85f2ea 100644 --- a/WebKitTools/Scripts/modules/buildbot_unittest.py +++ b/WebKitTools/Scripts/modules/buildbot_unittest.py @@ -91,6 +91,22 @@ class BuildBotTest(unittest.TestCase): for key, expected_value in expected_parsing.items(): self.assertEquals(builder[key], expected_value, ("Builder %d parse failure for key: %s: Actual='%s' Expected='%s'" % (x, key, builder[key], expected_value))) + def test_core_builder_methods(self): + buildbot = BuildBot() + + # Override builder_statuses function to not touch the network. + def example_builder_statuses(): # We could use instancemethod() to bind 'self' but we don't need to. + return BuildBotTest._expected_example_one_box_parsings + buildbot.builder_statuses = example_builder_statuses + + buildbot.core_builder_names_regexps = [ 'Leopard', "Windows.*Build" ] + self.assertEquals(buildbot.red_core_builders_names(), []) + self.assertTrue(buildbot.core_builders_are_green()) + + buildbot.core_builder_names_regexps = [ 'SnowLeopard', 'Qt' ] + self.assertEquals(buildbot.red_core_builders_names(), [ u'SnowLeopard Intel Release', u'Qt Linux Release' ]) + self.assertFalse(buildbot.core_builders_are_green()) + def test_builder_name_regexps(self): buildbot = BuildBot() diff --git a/WebKitTools/Scripts/modules/committers.py b/WebKitTools/Scripts/modules/committers.py index e157fb4..fc263eb 100644 --- a/WebKitTools/Scripts/modules/committers.py +++ b/WebKitTools/Scripts/modules/committers.py @@ -42,22 +42,28 @@ class Reviewer(Committer): Committer.__init__(self, name, email) self.can_review = True -# All reviewers are committers, so this list is only of committers -# who are not reviewers. +# This is intended as a cannonical, machine-readable list of all non-reviewer committers for WebKit. +# If your name is missing here and you are a committer, please add it. No review needed. +# All reviewers are committers, so this list is only of committers who are not reviewers. committers_unable_to_review = [ Committer("Aaron Boodman", "aa@chromium.org"), Committer("Adam Langley", "agl@chromium.org"), Committer("Albert J. Wong", "ajwong@chromium.org"), + Committer("Anton Muhin", "antonm@chromium.org"), Committer("Antonio Gomes", "tonikitoo@webkit.org"), Committer("Anthony Ricaud", "rik@webkit.org"), Committer("Ben Murdoch", "benm@google.com"), + Committer("Chris Fleizach", "cfleizach@apple.com"), Committer("Brent Fulgham", "bfulgham@webkit.org"), Committer("Brian Weinstein", "bweinstein@apple.com"), Committer("Cameron McCormack", "cam@webkit.org"), + Committer("Collin Jackson", "collinj@webkit.org"), + Committer("Csaba Osztrogonac", "ossy@webkit.org"), Committer("Daniel Bates", "dbates@webkit.org"), Committer("Drew Wilson", "atwilson@chromium.org"), Committer("Dirk Schulze", "krit@webkit.org"), Committer("Dmitry Titov", "dimich@chromium.org"), + Committer("Dumitru Daniliuc", "dumi@chromium.org"), Committer("Eli Fidler", "eli@staikos.net"), Committer("Eric Roman", "eroman@chromium.org"), Committer("Fumitoshi Ukai", "ukai@chromium.org"), @@ -69,38 +75,53 @@ committers_unable_to_review = [ Committer("Joseph Pecoraro", "joepeck@webkit.org"), Committer("Julie Parent", "jparent@google.com"), Committer("Kenneth Rohde Christiansen", "kenneth@webkit.org"), + Committer("Kent Tamura", "tkent@chromium.org"), Committer("Laszlo Gombos", "laszlo.1.gombos@nokia.com"), + Committer("Mads Ager", "ager@chromium.org"), + Committer("Mike Belshe", "mike@belshe.com"), Committer("Nate Chapin", "japhet@chromium.org"), Committer("Ojan Vafai", "ojan@chromium.org"), Committer("Pam Greene", "pam@chromium.org"), Committer("Peter Kasting", "pkasting@google.com"), Committer("Pierre d'Herbemont", "pdherbemont@free.fr"), + Committer("Roland Steiner", "rolandsteiner@chromium.org"), Committer("Ryosuke Niwa", "rniwa@webkit.org"), Committer("Scott Violet", "sky@chromium.org"), Committer("Shinichiro Hamaji", "hamaji@chromium.org"), + Committer("Steve Block", "steveblock@google.com"), Committer("Tony Chang", "tony@chromium.org"), Committer("Yael Aharon", "yael.aharon@nokia.com"), Committer("Yong Li", "yong.li@torchmobile.com"), + Committer("Yury Semikhatsky", "yurys@chromium.org"), Committer("Zoltan Horvath", "zoltan@webkit.org"), ] +# This is intended as a cannonical, machine-readable list of all reviewers for WebKit. +# If your name is missing here and you are a reviewer, please add it. No review needed. reviewers_list = [ Reviewer("Adam Barth", "abarth@webkit.org"), + Reviewer("Ada Chan", "adachan@apple.com"), Reviewer("Adam Roben", "aroben@apple.com"), Reviewer("Adam Treat", "treat@kde.org"), Reviewer("Adele Peterson", "adele@apple.com"), Reviewer("Alexey Proskuryakov", "ap@webkit.org"), + Reviewer("Alice Liu", "alice.liu@apple.com"), + Reviewer("Alp Toker", "alp@nuanti.com"), Reviewer("Anders Carlsson", "andersca@apple.com"), Reviewer("Antti Koivisto", "koivisto@iki.fi"), Reviewer("Ariya Hidayat", "ariya.hidayat@trolltech.com"), Reviewer("Brady Eidson", "beidson@apple.com"), + Reviewer("Cameron Zwarich", "zwarich@apple.com"), Reviewer("Dan Bernstein", "mitz@webkit.org"), Reviewer("Darin Adler", "darin@apple.com"), Reviewer("Darin Fisher", "fishd@chromium.org"), + Reviewer("David Harrison", "harrison@apple.com"), Reviewer("David Hyatt", "hyatt@apple.com"), Reviewer("David Kilzer", "ddkilzer@webkit.org"), Reviewer("David Levin", "levin@chromium.org"), Reviewer("Dimitri Glazkov", "dglazkov@chromium.org"), + Reviewer("Don Melton", "gramps@apple.com"), + Reviewer("Dmitri Titov", "dimich@chromium.org"), Reviewer("Eric Carlson", "eric.carlson@apple.com"), Reviewer("Eric Seidel", "eric@webkit.org"), Reviewer("Gavin Barraclough", "barraclough@apple.com"), @@ -110,16 +131,22 @@ reviewers_list = [ Reviewer("Holger Freyther", "zecke@selfish.org"), Reviewer("Jan Alonzo", "jmalonzo@gmail.com"), Reviewer("John Sullivan", "sullivan@apple.com"), + Reviewer("Jon Honeycutt", "jhoneycutt@apple.com"), Reviewer("Justin Garcia", "justin.garcia@apple.com"), + Reviewer("Kevin Decker", "kdecker@apple.com"), Reviewer("Kevin McCullough", "kmccullough@apple.com"), Reviewer("Kevin Ollivier", "kevino@theolliviers.com"), + Reviewer("Lars Knoll", "lars@trolltech.com"), Reviewer("Maciej Stachowiak", "mjs@apple.com"), Reviewer("Mark Rowe", "mrowe@apple.com"), Reviewer("Nikolas Zimmermann", "zimmermann@kde.org"), Reviewer("Oliver Hunt", "oliver@apple.com"), + Reviewer("Pavel Feldman", "pfeldman@chromium.org"), + Reviewer("Rob Buis", "rwlbuis@gmail.com"), Reviewer("Sam Weinig", "sam@webkit.org"), Reviewer("Simon Fraser", "simon.fraser@apple.com"), Reviewer("Simon Hausmann", "hausmann@webkit.org"), + Reviewer("Stephanie Lewis", "slewis@apple.com"), Reviewer("Steve Falkenburg", "sfalken@apple.com"), Reviewer("Timothy Hatcher", "timothy@hatcher.name"), Reviewer(u'Tor Arne Vestb\xf8', "vestbo@webkit.org"), diff --git a/WebKitTools/Scripts/modules/cpp_style.py b/WebKitTools/Scripts/modules/cpp_style.py index 0c9dfa0..485b07c 100644 --- a/WebKitTools/Scripts/modules/cpp_style.py +++ b/WebKitTools/Scripts/modules/cpp_style.py @@ -1533,16 +1533,15 @@ def check_spacing(filename, clean_lines, line_number, error): # Don't try to do spacing checks for operator methods line = re.sub(r'operator(==|!=|<|<<|<=|>=|>>|>)\(', 'operator\(', line) - - # We allow no-spaces around = within an if: "if ( (a=Foo()) == 0 )". - # Otherwise not. Note we only check for non-spaces on *both* sides; - # sometimes people put non-spaces on one side when aligning ='s among - # many lines (not that this is behavior that I approve of...) - if search(r'[\w.]=[\w.]', line) and not search(r'\b(if|while) ', line): + # Don't try to do spacing checks for #include statements at minimum it + # messes up checks for spacing around / + if match(r'\s*#\s*include', line): + return + if search(r'[\w.]=[\w.]', line): error(filename, line_number, 'whitespace/operators', 4, 'Missing spaces around =') - # FIXME: It's not ok to have spaces around binary operators like + - * / . + # FIXME: It's not ok to have spaces around binary operators like . # You should always have whitespace around binary operators. # Alas, we can't test < or > because they're legitimately used sans spaces @@ -1559,12 +1558,6 @@ def check_spacing(filename, clean_lines, line_number, error): if matched: error(filename, line_number, 'whitespace/operators', 3, 'Missing spaces around %s' % matched.group(1)) - # We allow no-spaces around << and >> when used like this: 10<<20, but - # not otherwise (particularly, not when used as streams) - matched = search(r'[^0-9\s](<<|>>)[^0-9\s=]', line) - if matched: - error(filename, line_number, 'whitespace/operators', 3, - 'Missing spaces around %s' % matched.group(1)) # There shouldn't be space around unary operators matched = search(r'(!\s|~\s|[\s]--[\s;]|[\s]\+\+[\s;])', line) @@ -1699,50 +1692,32 @@ def check_namespace_indentation(filename, clean_lines, line_number, file_extensi if not namespace_match: return - namespace_indentation = namespace_match.group('namespace_indentation') - - is_header_file = file_extension == 'h' - is_implementation_file = not is_header_file + current_indentation_level = len(namespace_match.group('namespace_indentation')) + if current_indentation_level > 0: + error(filename, line_number, 'whitespace/indent', 4, + 'namespace should never be indented.') + return + looking_for_semicolon = False; line_offset = 0 - - if is_header_file: - inner_indentation = namespace_indentation + ' ' * 4 - - for current_line in clean_lines.raw_lines[line_number + 1:]: - line_offset += 1 - - # Skip not only empty lines but also those with preprocessor directives. - # Goto labels don't occur in header files, so no need to check for those. - if current_line.strip() == '' or current_line.startswith('#'): - continue - - if not current_line.startswith(inner_indentation): - # If something unindented was discovered, make sure it's a closing brace. - if not current_line.startswith(namespace_indentation + '}'): + in_preprocessor_directive = False; + for current_line in clean_lines.elided[line_number + 1:]: + line_offset += 1 + if not current_line.strip(): + continue + if not current_indentation_level: + if not (in_preprocessor_directive or looking_for_semicolon): + if not match(r'\S', current_line): error(filename, line_number + line_offset, 'whitespace/indent', 4, - 'In a header, code inside a namespace should be indented.') - break - - if is_implementation_file: - for current_line in clean_lines.raw_lines[line_number + 1:]: - line_offset += 1 - - # Skip not only empty lines but also those with (goto) labels. - # The goto label regexp accepts spaces or the beginning of a - # comment (if anything) after the initial colon. - if current_line.strip() == '' or match(r'\w+\s*:([\s\/].*)?$', current_line): - continue - - remaining_line = current_line[len(namespace_indentation):] - if not match(r'\S', remaining_line): - error(filename, line_number + line_offset, 'whitespace/indent', 4, - 'In an implementation file, code inside a namespace should not be indented.') - - # Just check the first non-empty line in any case, because - # otherwise we would need to count opened and closed braces, - # which is obviously a lot more complicated. - break - + 'Code inside a namespace should not be indented.') + if in_preprocessor_directive or (current_line.strip()[0] == '#'): # This takes care of preprocessor directive syntax. + in_preprocessor_directive = current_line[-1] == '\\' + else: + looking_for_semicolon = ((current_line.find(';') == -1) and (current_line.strip()[-1] != '}')) or (current_line[-1] == '\\') + else: + looking_for_semicolon = False; # If we have a brace we may not need a semicolon. + current_indentation_level += current_line.count('{') - current_line.count('}') + if current_indentation_level < 0: + break; def check_using_std(filename, clean_lines, line_number, error): """Looks for 'using std::foo;' statements which should be replaced with 'using namespace std;'. diff --git a/WebKitTools/Scripts/modules/cpp_style_unittest.py b/WebKitTools/Scripts/modules/cpp_style_unittest.py index 322356e..d5637f4 100644 --- a/WebKitTools/Scripts/modules/cpp_style_unittest.py +++ b/WebKitTools/Scripts/modules/cpp_style_unittest.py @@ -1265,6 +1265,12 @@ class CppStyleTest(CppStyleTestBase): self.assert_lint('a<Foo&> t <<= &b | &c;', '') self.assert_lint('a<Foo*> t <<= &b & &c; // Test', '') self.assert_lint('a<Foo*> t <<= *b / &c; // Test', '') + self.assert_lint('if (a=b == 1)', 'Missing spaces around = [whitespace/operators] [4]') + self.assert_lint('a = 1<<20', 'Missing spaces around << [whitespace/operators] [3]') + self.assert_lint('if (a = b == 1)', '') + self.assert_lint('a = 1 << 20', '') + self.assert_multi_line_lint('#include "config.h"\n#include <sys/io.h>\n', + '') def test_spacing_before_last_semicolon(self): self.assert_lint('call_function() ;', @@ -2806,40 +2812,39 @@ class WebKitStyleTest(CppStyleTestBase): 'Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]') # FIXME: No tests for 8-spaces. - # 3. In a header, code inside a namespace should be indented. + # 3. In a header, code inside a namespace should not be indented. self.assert_multi_line_lint( 'namespace WebCore {\n\n' - ' class Document {\n' - ' int myVariable;\n' - ' };\n' + 'class Document {\n' + ' int myVariable;\n' + '};\n' '}', '', 'foo.h') self.assert_multi_line_lint( 'namespace OuterNamespace {\n' ' namespace InnerNamespace {\n' - ' class Document {\n' - ' };\n' - ' };\n' + ' class Document {\n' + '};\n' + '};\n' '}', - '', + ['Code inside a namespace should not be indented. [whitespace/indent] [4]', 'namespace should never be indented. [whitespace/indent] [4]'], 'foo.h') self.assert_multi_line_lint( 'namespace WebCore {\n' '#if 0\n' ' class Document {\n' - ' };\n' + '};\n' '#endif\n' '}', - '', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', 'foo.h') self.assert_multi_line_lint( 'namespace WebCore {\n' 'class Document {\n' '};\n' '}', - 'In a header, code inside a namespace should be indented.' - ' [whitespace/indent] [4]', + '', 'foo.h') # 4. In an implementation file (files with the extension .cpp, .c @@ -2858,14 +2863,52 @@ class WebKitStyleTest(CppStyleTestBase): 'namespace OuterNamespace {\n' 'namespace InnerNamespace {\n' 'Document::Foo() { }\n' - '}', - '', + ' void* p;\n' + '}\n' + '}\n', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', 'foo.cpp') self.assert_multi_line_lint( - ' namespace WebCore {\n\n' - 'start: // Pointless code, but tests the label regexp.\n' - ' goto start;\n' - ' }', + 'namespace OuterNamespace {\n' + 'namespace InnerNamespace {\n' + 'Document::Foo() { }\n' + '}\n' + ' void* p;\n' + '}\n', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', + 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n\n' + ' const char* foo = "start:;"\n' + ' "dfsfsfs";\n' + '}\n', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', + 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n\n' + 'const char* foo(void* a = ";", // ;\n' + ' void* b);\n' + ' void* p;\n' + '}\n', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', + 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n\n' + 'const char* foo[] = {\n' + ' "void* b);", // ;\n' + ' "asfdf",\n' + ' }\n' + ' void* p;\n' + '}\n', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', + 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n\n' + 'const char* foo[] = {\n' + ' "void* b);", // }\n' + ' "asfdf",\n' + ' }\n' + '}\n', '', 'foo.cpp') self.assert_multi_line_lint( @@ -2875,15 +2918,30 @@ class WebKitStyleTest(CppStyleTestBase): 'start: // infinite loops are fun!\n' ' goto start;\n' ' }', - '', + 'namespace should never be indented. [whitespace/indent] [4]', 'foo.cpp') self.assert_multi_line_lint( 'namespace WebCore {\n' ' Document::Foo() { }\n' '}', - 'In an implementation file, code inside a namespace should not be indented.' + 'Code inside a namespace should not be indented.' ' [whitespace/indent] [4]', 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n' + '#define abc(x) x; \\\n' + ' x\n' + '}', + '', + 'foo.cpp') + self.assert_multi_line_lint( + 'namespace WebCore {\n' + '#define abc(x) x; \\\n' + ' x\n' + ' void* x;' + '}', + 'Code inside a namespace should not be indented. [whitespace/indent] [4]', + 'foo.cpp') # 5. A case label should line up with its switch statement. The # case statement is indented. @@ -3246,7 +3304,7 @@ class WebKitStyleTest(CppStyleTestBase): '') self.assert_multi_line_lint( 'namespace WebCore {\n' - ' int foo;\n' + 'int foo;\n' '};\n', '') self.assert_multi_line_lint( diff --git a/WebKitTools/Scripts/modules/scm.py b/WebKitTools/Scripts/modules/scm.py index 3daecbc..3ffa23b 100644 --- a/WebKitTools/Scripts/modules/scm.py +++ b/WebKitTools/Scripts/modules/scm.py @@ -124,9 +124,14 @@ class SCM: @staticmethod def run_command(args, cwd=None, input=None, error_handler=default_error_handler, return_exit_code=False): - stdin = subprocess.PIPE if input else None + if hasattr(input, 'read'): # Check if the input is a file. + stdin = input + string_to_communicate = None + else: + stdin = subprocess.PIPE if input else None + string_to_communicate = input process = subprocess.Popen(args, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd) - output = process.communicate(input)[0].rstrip() + output = process.communicate(string_to_communicate)[0].rstrip() exit_code = process.wait() if exit_code: script_error = ScriptError(script_args=args, exit_code=exit_code, output=output, cwd=cwd) @@ -166,11 +171,8 @@ class SCM: args = [self.script_path('svn-apply'), '--reviewer', patch['reviewer']] if force: args.append('--force') - patch_apply_process = subprocess.Popen(args, stdin=curl_process.stdout) - return_code = patch_apply_process.wait() - if return_code: - raise ScriptError(message="Patch %s from bug %s failed to download and apply." % (patch['url'], patch['bug_id'])) + self.run_command(args, input=curl_process.stdout) def run_status_and_extract_filenames(self, status_command, status_regexp): filenames = [] @@ -392,7 +394,7 @@ class Git(SCM): @classmethod def in_working_directory(cls, path): - return cls.run_command(['git', 'rev-parse', '--is-inside-work-tree'], cwd=path) == "true" + return cls.run_command(['git', 'rev-parse', '--is-inside-work-tree'], cwd=path, error_handler=ignore_error) == "true" @classmethod def find_checkout_root(cls, path): diff --git a/WebKitTools/Scripts/modules/scm_unittest.py b/WebKitTools/Scripts/modules/scm_unittest.py index 58494a0..784303f 100644 --- a/WebKitTools/Scripts/modules/scm_unittest.py +++ b/WebKitTools/Scripts/modules/scm_unittest.py @@ -95,8 +95,6 @@ class SVNTestRepository: @classmethod def setup(cls, test_object): - test_object.original_path = os.path.abspath('.') - # Create an test SVN repository test_object.svn_repo_path = tempfile.mkdtemp(suffix="svn_test_repo") test_object.svn_repo_url = "file://%s" % test_object.svn_repo_path # Not sure this will work on windows @@ -115,23 +113,30 @@ class SVNTestRepository: run(['rm', '-rf', test_object.svn_repo_path]) run(['rm', '-rf', test_object.svn_checkout_path]) +# For testing the SCM baseclass directly. +class SCMClassTests(unittest.TestCase): + def setUp(self): + self.dev_null = open(os.devnull, "w") # Used to make our Popen calls quiet. -class SCMTest(unittest.TestCase): - def _create_patch(self, patch_contents): - patch_path = os.path.join(self.svn_checkout_path, 'patch.diff') - write_into_file_at_path(patch_path, patch_contents) - patch = {} - patch['reviewer'] = 'Joe Cool' - patch['bug_id'] = '12345' - patch['url'] = 'file://%s' % urllib.pathname2url(patch_path) - return patch + def tearDown(self): + self.dev_null.close() - def _setup_webkittools_scripts_symlink(self, local_scm): - webkit_scm = detect_scm_system(self.original_path) - webkit_scripts_directory = webkit_scm.scripts_directory() - local_scripts_directory = local_scm.scripts_directory() - os.mkdir(os.path.dirname(local_scripts_directory)) - os.symlink(webkit_scripts_directory, local_scripts_directory) + def test_run_command_with_pipe(self): + input_process = subprocess.Popen(['/bin/echo', 'foo\nbar'], stdout=subprocess.PIPE, stderr=self.dev_null) + self.assertEqual(SCM.run_command(['/usr/bin/grep', 'bar'], input=input_process.stdout), "bar") + + # Test the non-pipe case too: + self.assertEqual(SCM.run_command(['/usr/bin/grep', 'bar'], input="foo\nbar"), "bar") + + command_returns_non_zero = ['/bin/sh', '--invalid-option'] + # Test when the input pipe process fails. + input_process = subprocess.Popen(command_returns_non_zero, stdout=subprocess.PIPE, stderr=self.dev_null) + self.assertTrue(input_process.poll() != 0) + self.assertRaises(ScriptError, SCM.run_command, ['/usr/bin/grep', 'bar'], input=input_process.stdout) + + # Test when the run_command process fails. + input_process = subprocess.Popen(['/bin/echo', 'foo\nbar'], stdout=subprocess.PIPE, stderr=self.dev_null) # grep shows usage and calls exit(2) when called w/o arguments. + self.assertRaises(ScriptError, SCM.run_command, command_returns_non_zero, input=input_process.stdout) def test_error_handlers(self): git_failure_message="Merge conflict during commit: Your file or directory 'WebCore/ChangeLog' is probably out-of-date: resource out of date; try updating at /usr/local/libexec/git-core//git-svn line 469" @@ -152,6 +157,24 @@ svn: resource out of date; try updating self.assertRaises(ScriptError, commit_error_handler, ScriptError(output='blah blah blah')) +# GitTest and SVNTest inherit from this so any test_ methods here will be run once for this class and then once for each subclass. +class SCMTest(unittest.TestCase): + def _create_patch(self, patch_contents): + patch_path = os.path.join(self.svn_checkout_path, 'patch.diff') + write_into_file_at_path(patch_path, patch_contents) + patch = {} + patch['reviewer'] = 'Joe Cool' + patch['bug_id'] = '12345' + patch['url'] = 'file://%s' % urllib.pathname2url(patch_path) + return patch + + def _setup_webkittools_scripts_symlink(self, local_scm): + webkit_scm = detect_scm_system(os.path.dirname(os.path.abspath(__file__))) + webkit_scripts_directory = webkit_scm.scripts_directory() + local_scripts_directory = local_scm.scripts_directory() + os.mkdir(os.path.dirname(local_scripts_directory)) + os.symlink(webkit_scripts_directory, local_scripts_directory) + # Tests which both GitTest and SVNTest should run. # FIXME: There must be a simpler way to add these w/o adding a wrapper method to both subclasses def _shared_test_commit_with_message(self): @@ -188,7 +211,6 @@ class SVNTest(SCMTest): def tearDown(self): SVNTestRepository.tear_down(self) - os.chdir(self.original_path) def test_create_patch_is_full_patch(self): test_dir_path = os.path.join(self.svn_checkout_path, 'test_dir') @@ -284,7 +306,6 @@ class GitTest(SCMTest): def tearDown(self): SVNTestRepository.tear_down(self) self._tear_down_git_clone_of_svn_repository() - os.chdir(self.original_path) def test_detection(self): scm = detect_scm_system(self.git_checkout_path) |