1137442 - Try server email doesn't handle quotes properly

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Andrew McCreight [:mccr8]]

I did a try push where the top three commits were like so:

 * "try: -b d -p linux -u all -t none"
 * Bug 1119482, part 4 - Move IsJSHolder into HoldDropJSObjects. r=smaug
 * Bug 1119482, part 3 - Move the Hold and DropJSObjects implementations to a new

In the actual try push, it seems to have recognized the try syntax, despite it being in quotes, so that's good.  However, the email it sent to me had the subject:

try submission c54ebf460307: "

That's it.  I guess there are two issues.  One is that it should recognize that this is try syntax and print out the commit message for the next commit, like it normally does.  Secondly, if there are quotes, don't just throw out the rest of the commit message.

Comment 1

[Andrew McCreight [:mccr8]]

here's what it looks like on treeherder if that helps: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c54ebf460307

Comment 2

[Ed Morley [:emorley]]

Need to add quotes to:
https://hg.mozilla.org/build/buildbotcustom/file/30f690bd597f/status/generators.py#l54

The |re.sub(r'\btry: .*', '', title)| took off everything but the opening quote, and then because we didn't strip that, 'title' was non-empty, so we used that rather than trying the next line.

Comment 3

[Ed Morley [:emorley]]

Attachment: Try to use a try email commit description of at least 3 characters

When iterating through the list of commits in a try push to extract a
summary for the email, reject titles that are less than 3 characters.

I've gone with this approach rather than the one in the previous comment, since if we did add quotes to the list of trailing punctuation to remove, then this commit:
Bug 123 - Fix the 'foo'

Would become:
Bug 123 - Fix the 'foo

Ensuring a minimum length likely also prevents a few other unhelpful cases, so I think it's preferable.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8570383 [details] [diff] [review]
Try to use a try email commit description of at least 3 characters

Review of attachment 8570383 [details] [diff] [review]:
-----------------------------------------------------------------

Please update test/test_status.py with a working test for this case before landing. We run tests with tox.

Comment 5

[Ed Morley [:emorley]]

(In reply to Nick Thomas [:nthomas] from comment #4)
> Please update test/test_status.py with a working test for this case before
> landing. We run tests with tox.

Done - thanks for the review :-)

remote:   https://hg.mozilla.org/build/buildbotcustom/rev/79b669d659b0

Comment 6

[Jordan Lund (:jlund)]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/79b669d659b0