diff --git a/rbtools/clients/__init__.py b/rbtools/clients/__init__.py index 71aef48bea019168a62e459d95f24f516df8711d..33ad059da4b4e421fd87d915dfc9583eb221edf7 100644 --- a/rbtools/clients/__init__.py +++ b/rbtools/clients/__init__.py @@ -264,10 +264,20 @@ class SCMClient(object): raise NotImplementedError def get_commit_message(self, revisions): - """Returns the commit message from the commits in the given revisions. + """Return the commit message from the commits in the given revisions. - This pulls out the first line from the commit messages of the - given revisions. That is then used as the summary. + The first line of the commit messages of the given revisions will be + used as the summary while the rest of the commit message will be + parsed out for the description and testing done. + + Args: + revisions (dict): + A revisions dictionary as returned by ``parse_revision_spec``. + + Returns: + dict: + A dictionary containing the new summary, description and testing + done fields obtained from revisions. """ commit_message = self.get_raw_commit_message(revisions) lines = commit_message.splitlines() @@ -276,15 +286,37 @@ class SCMClient(object): return None result = { - 'summary': lines[0], + 'summary': lines[0].strip(), + 'description': '', + 'testing_done': '', } + r = re.compile(r'testing done:?$', re.I) - # Try to pull the body of the commit out of the full commit - # description, so that we can skip the summary. if len(lines) >= 3 and lines[0] and not lines[1]: - result['description'] = '\n'.join(lines[2:]).strip() + # We have a clear "Summary\n\nDescription"-style commit message so + # we can split out the description. + description = lines[2:] else: - result['description'] = commit_message + # We do not have a clear "Summary\n\nDescription"-style commit + # message so we assume everything is the description. + description = lines + + if description: + for i, line in enumerate(description): + if r.match(line): + result['testing_done'] = \ + '\n'.join(description[i + 1:]).strip() + + if i == 0: + # The entire description is the testing done, so + # the resulting description will be empty. + description = [] + else: + description = description[:i - 1] + + break + + result['description'] = '\n'.join(description).strip() return result diff --git a/rbtools/clients/tests/test_base.py b/rbtools/clients/tests/test_base.py new file mode 100644 index 0000000000000000000000000000000000000000..e577cb99873b5a580a1bdc1fa9d93d82ef7e7491 --- /dev/null +++ b/rbtools/clients/tests/test_base.py @@ -0,0 +1,187 @@ +"""Unit tests for rbtools.clients.SCMClient.""" + +from __future__ import unicode_literals + +from kgb import SpyAgency + +from rbtools.clients import SCMClient +from rbtools.clients.tests import SCMClientTests + + +class SCMBaseClientTests(SpyAgency, SCMClientTests): + """Unit tests for rbtools.clients.SCMClient.""" + + def setUp(self): + super(SCMBaseClientTests, self).setUp() + self.client = SCMClient() + + def test_get_commit_message_with_summary_and_testing_done(self): + """Testing SCMClient.get_commit_message with only summary and testing + done in the commit message + """ + commit_message = ( + b'This is the summary.\n' + b'\n' + b'Testing Done:\n' + b'This is the testing done portion of the commit message.\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'This is the summary.', + 'description': '', + 'testing_done': 'This is the testing done portion of the ' + 'commit message.', + }) + + def test_get_commit_message_with_summary_description_and_testing(self): + """Testing SCMClient.get_commit_message with summary, description and + testing done in the commit message + """ + commit_message = ( + b'This is the summary.\n' + b'\n' + b'This is the description portion of the commit message. I will ' + b'add more text to take up at least two lines since most ' + b'descriptions will be longer than one line.\n' + b'\n' + b'Testing Done:\n' + b'This is the testing done portion of the commit message.\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'This is the summary.', + 'description': 'This is the description portion of the commit ' + 'message. I will add more text to take up at ' + 'least two lines since most descriptions will ' + 'be longer than one line.', + 'testing_done': 'This is the testing done portion of the ' + 'commit message.' + }) + + def test_get_commit_message_with_summary_and_description(self): + """Testing SCMClient.get_commit_message with only summary and + description in the commit message + """ + commit_message = ( + b'This is the summary.\n' + b'\n' + b'This is the description portion of the commit message. I will ' + b'add more text to take up at least two lines since most ' + b'descriptions will be longer than one line.\n' + b'\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'This is the summary.', + 'description': 'This is the description portion of the commit ' + 'message. I will add more text to take up at ' + 'least two lines since most descriptions will ' + 'be longer than one line.', + 'testing_done': '', + }) + + def test_get_commit_message_with_only_summary(self): + """Testing SCMClient.get_commit_message with only summary in the + commit message + """ + commit_message = ( + b'This is the summary.\n' + b'\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'This is the summary.', + 'description': 'This is the summary.', + 'testing_done': '', + }) + + def test_get_commit_message_with_summary_and_description_together(self): + """Testing SCMClient.get_commit_message with summary and + description without a blank line in between the two + """ + commit_message = ( + b'This is the summary.\n' + b'This is the description portion of the commit message. I will ' + b'add more text to take up at least two lines since most ' + b'descriptions will be longer than one line.\n' + b'\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'This is the summary.', + 'description': 'This is the summary.\nThis is the description ' + 'portion of the commit message. I will add more' + ' text to take up at least two lines since most' + ' descriptions will be longer than one line.', + 'testing_done': '' + }) + + def test_get_commit_message_with_testing_done(self): + """Testing SCMClient.get_commit_message with only testing done in + the commit message + """ + commit_message = ( + b'Testing Done:\n' + b'This is the testing portion of the commit message.\n' + b'\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'Testing Done:', + 'description': '', + 'testing_done': 'This is the testing portion of the ' + 'commit message.' + }) + + def test_get_commit_message_with_multiple_testing_done(self): + """Testing SCMClient.get_commit_message with multiple `testing done` in + the commit message + """ + commit_message = ( + b'Testing Done:\n' + b'Testing Done:\n' + b'This is the testing portion of the commit message.\n' + b'\n' + ) + + self.spy_on(self.client.get_raw_commit_message, + call_fake=lambda *args: commit_message) + + self.assertEqual( + self.client.get_commit_message([]), + { + 'summary': 'Testing Done:', + 'description': '', + 'testing_done': 'Testing Done:\nThis is the testing portion ' + 'of the commit message.' + }) diff --git a/rbtools/commands/post.py b/rbtools/commands/post.py index cb93c6140a18e64cf3f05a90b08048c5577e780a..139526922826d056d971266d0eca7a7208af0717 100644 --- a/rbtools/commands/post.py +++ b/rbtools/commands/post.py @@ -149,6 +149,21 @@ class Post(Command): 'guessing behavior. See :ref:`guessing-behavior` ' 'for more information.' )), + Option('--guess-testing-done', + dest='guess_testing_done', + action='store', + config_key='GUESS_TESTING_FIELDS', + nargs='?', + default=None, + const=GUESS_YES, + choices=GUESS_CHOICES, + help='Generate the Testing Done field based on the ' + 'commit messages.', + extended_help=( + 'This can optionally take a value to control the ' + 'guessing behavior. See :ref:`guessing-behavior` ' + 'for more information.', + )), Option('--guess-summary', dest='guess_summary', action='store', @@ -313,11 +328,13 @@ class Post(Command): 'TARGET_PEOPLE' in self.config): self.options.target_people = self.config['TARGET_PEOPLE'] - # -g implies --guess-summary and --guess-description + # -g implies --guess-summary, --guess-description and + # guess-testing-done. self.options.guess_fields = self.normalize_guess_value( self.options.guess_fields, '--guess-fields') - for field_name in ('guess_summary', 'guess_description'): + for field_name in ('guess_summary', 'guess_description', + 'guess_testing_done'): # We want to ensure we only override --guess-{field} with # --guess-fields when --guess-{field} is not provided. # to the default (auto). @@ -337,12 +354,12 @@ class Post(Command): 'Subversion changelist, pass the changelist name as an ' 'additional argument after the command.') - # Only one of --description and --description-file can be used + # Only one of --description and --description-file can be used. if self.options.description and self.options.description_file: raise CommandError('The --description and --description-file ' 'options are mutually exclusive.') - # If --description-file is used, read that file + # If --description-file is used, read that file. if self.options.description_file: if os.path.exists(self.options.description_file): with open(self.options.description_file, 'r') as fp: @@ -352,12 +369,12 @@ class Post(Command): 'The description file %s does not exist.' % self.options.description_file) - # Only one of --testing-done and --testing-done-file can be used + # Only one of --testing-done and --testing-done-file can be used. if self.options.testing_done and self.options.testing_file: raise CommandError('The --testing-done and --testing-done-file ' 'options are mutually exclusive.') - # If --testing-done-file is used, read that file + # If --testing-done-file is used, read that file. if self.options.testing_file: if os.path.exists(self.options.testing_file): with open(self.options.testing_file, 'r') as fp: @@ -367,7 +384,7 @@ class Post(Command): % self.options.testing_file) # If we have an explicitly specified summary, override - # --guess-summary + # --guess-summary. if self.options.summary: self.options.guess_summary = self.GUESS_NO else: @@ -375,13 +392,21 @@ class Post(Command): self.options.guess_summary, '--guess-summary') # If we have an explicitly specified description, override - # --guess-description + # --guess-description. if self.options.description: self.options.guess_description = self.GUESS_NO else: self.options.guess_description = self.normalize_guess_value( self.options.guess_description, '--guess-description') + # If we have an explicitly specified testing_done, override + # --guess-testing-done. + if self.options.testing_done: + self.options.guess_testing_done = self.GUESS_NO + else: + self.options.guess_testing_done = self.normalize_guess_value( + self.options.guess_testing_done, '--guess-testing-done') + # If the --diff-filename argument is used, we can't do automatic # updating. if self.options.diff_filename and self.options.update: @@ -389,7 +414,7 @@ class Post(Command): 'using --diff-filename.') # If we have an explicitly specified review request ID, override - # --update + # --update. if self.options.rid and self.options.update: self.options.update = False @@ -595,11 +620,11 @@ class Post(Command): update_fields['trivial'] = True if not self.options.diff_only: - # If the user has requested to guess the summary or description, - # get the commit message and override the summary and description - # options, which we'll fill in below. The guessing takes place - # after stamping so that the guessed description matches the commit - # when rbt exits. + # If the user has requested to guess the summary, description, or + # testing_done, get the commit message and override the options for + # each guessed field, which we'll fill in below. The guessing takes + # place after stamping so that the guessed description matches the + # commit when rbt exits. if not self.options.diff_filename: self.check_guess_fields() @@ -676,6 +701,23 @@ class Post(Command): return review_request.id, review_request.absolute_url + def should_guess_field(self, field_value, is_new_review_request): + """Determine whether or not we should guess a given field. + + Args: + field_value (unicode): + The guess field to be checked. + + is_new_review_request (bool): + Whether or not we are posting a new review request. + Returns + bool: + Whether or not the given field should be guessed. + """ + return (field_value == self.GUESS_YES or + (field_value == self.GUESS_AUTO and + is_new_review_request)) + def check_guess_fields(self): """Checks and handles field guesses for the review request. @@ -690,38 +732,48 @@ class Post(Command): is_new_review_request = (not self.options.rid and not self.options.update) - guess_summary = ( - self.options.guess_summary == self.GUESS_YES or - (self.options.guess_summary == self.GUESS_AUTO and - is_new_review_request)) - guess_description = ( - self.options.guess_description == self.GUESS_YES or - (self.options.guess_description == self.GUESS_AUTO and - is_new_review_request)) + guess_summary = self.should_guess_field( + self.options.guess_summary, + is_new_review_request) - if self.revisions and (guess_summary or guess_description): + guess_description = self.should_guess_field( + self.options.guess_description, + is_new_review_request) + + guess_testing_done = self.should_guess_field( + self.options.guess_testing_done, + is_new_review_request) + + if self.revisions and (guess_summary or guess_description or + guess_testing_done): try: commit_message = self.tool.get_commit_message(self.revisions) if commit_message: guessed_summary = commit_message['summary'] guessed_description = commit_message['description'] + guessed_testing_done = commit_message['testing_done'] - if guess_summary and guess_description: + if guess_summary: self.options.summary = guessed_summary - self.options.description = guessed_description - elif guess_summary: - self.options.summary = guessed_summary - elif guess_description: - # If we're guessing the description but not the summary - # (for example, if --summary was included), we probably - # don't want to strip off the summary line of the - # commit message. - if guessed_description.startswith(guessed_summary): + + if guess_description: + if guess_summary: self.options.description = guessed_description else: - self.options.description = \ - guessed_summary + '\n\n' + guessed_description + # If we're guessing the description but not the + # summary (for example, if --summary was included), + # we probably don't want to strip off the summary + # line of the commit message. + if guessed_description.startswith(guessed_summary): + self.options.description = guessed_description + else: + self.options.description = \ + '%s\n\n%s' % (guessed_summary, + guessed_description) + + if guess_testing_done: + self.options.testing_done = guessed_testing_done except NotImplementedError: # The SCMClient doesn't support getting commit messages, # so we can't provide the guessed versions.