1278746 - Followup work for the bug filer tool

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

There's some things that were on the nice-to-have list, but didn't make the first cut for the bug filer:

1. Extract all of the bugzilla URLs on the UI side so they can all be set from a central point, like services/main.js (or maybe set up a new service).

2. Maybe a link to MXR/DXR for the file in question.

3. Maybe improve the initial product searching using https://hg.mozilla.org/mozilla-central/json-mozbuildinfo instead of a hardcoded object of products.

4. Add the ability to specify Platform/OS before filing the bug.

5. Add the ability to specify keywords. Right now, "intermittent-failure" is hardcoded and nothing else can be added. Maybe we can also provide an option to omit it in some instances in case anyone wants to re-use the bug filer for non-intermittent failures?

6. Right now, version gets hardcoded to whatever the api returns as the default version for the product. This can cause some issues for some products where the version field is not used (Camd had trouble trying to file a failure into the Treeherder product because the version wasn't returned correctly.) We should get that code a bit more robust, at the very least.

7. Ability to set flags. At the very least, the needinfo flag. Maybe we can also set the status flags for whatever version of Firefox is being used, so the bug automatically gets status-firefoxXX set to 'affected' when the bug gets filed?

8. The Cc list could be edited from the filer form.

9. Blocks/depends/seealso fields could be edited from the filer form.

10. We could extract crash signatures and fill in that field automatically also.



I'm sure there's more, but these would be a big step forward from what just landed.

Comment 1

[Ed Morley [:emorley]]

Some great ideas there!

++ on landing the MVP first and breaking these out to a new bug, will be much easier to iterate when people are using it in production :-)

Comment 2

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:1278746 > mozilla:master

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8769022 [details] [review]
[treeherder] KWierso:1278746 > mozilla:master

So far this does #2 from that list. 
MXR is decommissioned so this only does DXR links. If this repo is from an integration tree, a dxr link to mozilla-central is shown, since integration branches aren't indexed on dxr. 

Otherwise, the dxr link points to the repo that this bug is being filed from.

Additionally, if the bug is being filed from a release branch, additional dxr links are shown for the repos "newer" than the current repo. So a failure on aurora will show dxr links for aurora (current repo) and mozilla-central. Failure on beta also shows m-c, m-a and m-b. Release shows m-c, m-a, m-b and m-r.



Aside from the dxr links, this also hopefully caps summaries at 255 characters, preventing some issues tomcat's reported about getting errors from bugzilla about summaries being too long. Not sure how that's happening since the summary input box has a maxlength identical to bugzilla's new bug form, but maybe explicitly trimming it at 255 characters will help.



More to come. I can split these out to new bugs if that's easier to work with.

Comment 4

[Phil Ringnalda (:philor)]

I can live with having the summary chopped, but I didn't file it because I find I can file better bugs by editing it down myself (the addition of two 80% useless paths to reftest failure lines made them stupid, I can cut them down to sanity better than having the end cut off, etc.).

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Phil Ringnalda (:philor) from comment #4)
> I can live with having the summary chopped, but I didn't file it because I
> find I can file better bugs by editing it down myself (the addition of two
> 80% useless paths to reftest failure lines made them stupid, I can cut them
> down to sanity better than having the end cut off, etc.).

I guess a better solution might be to just ensure the summary length doesn't exceed 255 before passing the information over to the server to file the bug, bailing if it somehow manages to exceed 255?

Comment 6

[Phil Ringnalda (:philor)]

That's not a better solution for me, since I wouldn't know it was going to get chopped until it already had been, and I don't actually look at the filed bug.

A better solution for me would be to have a character count that only appears when it's over 255, and to not be willing to even try to submit until it gets down under. But that's what would be better for me.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Phil Ringnalda (:philor) from comment #6)
> That's not a better solution for me, since I wouldn't know it was going to
> get chopped until it already had been, and I don't actually look at the
> filed bug.
> 
> A better solution for me would be to have a character count that only
> appears when it's over 255, and to not be willing to even try to submit
> until it gets down under. But that's what would be better for me.

Consider it done. It always shows the character count, but it's green when 255 and under, and red when over 255. The input field also outlines in light red when over 255. And if you click Submit when it's over 255, it just throws a notification saying you need to cut it down.

Comment 8

[Ed Morley [:emorley]]

Comment on attachment 8769022 [details] [review]
[treeherder] KWierso:1278746 > mozilla:master

Left some comments :-)

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

> 3. Maybe improve the initial product searching using https://hg.mozilla.org/mozilla-central/json-mozbuildinfo instead of a hardcoded object of products.

Is mozbuildinfo still working? A previously working example URL is just responding with `error: "unable to obtain moz.build info"`

Comment 10

[Gregory Szorc [:gps]]

No, the mozbuildinfo URLs are broken. Bug 1263973. Make noise there if it needs to be unbusted.

Comment 11

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:showdxrlinks > mozilla:master

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Could you clarify your two comments about hardcoding things? I'm not sure I follow what you're asking.

Comment 13

[Ed Morley [:emorley]]

(In reply to Wes Kocher (:KWierso) from comment #12)
> Could you clarify your two comments about hardcoding things? I'm not sure I
> follow what you're asking.

I mean:
* Put constants such as URLs in a separate file, especially for things that are used multiple times
* Put numeric/string constants in values.js (eg list of repos that fall into a certain category)
* Use a more generic way of handling the different branches than having to have the naming structure hardcoded into several functions like `isAuroraOrOlder()`, `isReleaseOrOlder()` etc.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Something else nice to have would be if the bug filer tool would automatically trim the bug summary to the maximum allowed length for Bugzilla. In case of failures like the following I get an error:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=Firefox%20UI&filter-tier=1&filter-tier=2&filter-tier=3&bugfiler&selectedJob=4607456

> Bug Filer API returned status 400 (BAD REQUEST) The text you entered in the Summary field is too long (493 characters, above the maximum length allowed of 255 characters).

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

With this PR as-is, it will refuse to attempt to file (and will warn you ahead of time if you go too long) if the summary is too long. I'd rather not have to figure out how to auto-trim summaries in the bugfiler tool since that could easily cut off important information. Do you need more than that?

Comment 16

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:1278746skipversions > mozilla:master

Comment 17

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:1278746optionalintermittent > mozilla:master

Comment 18

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:1278746longsummaries > mozilla:master

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Ed, I've split out each individual change to a separate pull request so I can get some of these landed before fully working out some of the more involved changes. Would you prefer if I file a new dependent bug for each of these, or should I just flag you for the reviews in this bug?

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Wes Kocher (:KWierso) from comment #15)
> have to figure out how to auto-trim summaries in the bugfiler tool since
> that could easily cut off important information. Do you need more than that?

This makes sense. I think that this is the right choice. So all fine. Thanks.

Comment 21

[Ed Morley [:emorley]]

(In reply to Wes Kocher (:KWierso) from comment #19)
> Ed, I've split out each individual change to a separate pull request so I
> can get some of these landed before fully working out some of the more
> involved changes. Would you prefer if I file a new dependent bug for each of
> these, or should I just flag you for the reviews in this bug?

This bug is fine :-)

(If there are pieces left over that aren't going to be resolved for months, then we can always file separate bugs for them once the 90% has landed here).

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8779199 [details] [review]
[treeherder] KWierso:1278746skipversions > mozilla:master

If you need to test this one, I'd suggest attempting to file a bug in the Treeherder component. Without this patch, the filer will attempt to use the disabled version "trunk" and fail. With this patch, it will use the active version "---" and succeed.

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8779200 [details] [review]
[treeherder] KWierso:1278746optionalintermittent > mozilla:master

This should be a quick review. If you uncheck the new "intermittent" checkbox, the filer will send an empty "keywords" field. 

I suppose I could have it not send any keywords field if there's no keyword being set...

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8779201 [details] [review]
[treeherder] KWierso:1278746longsummaries > mozilla:master

This is the most involved of these three PRs. 

When you open the bug filer, it should show the character count of the bug summary off to the right of the summary field. If that count exceeds 255 characters, the count turns red and the summary field gets a red outline. Once the count drops to 255 or lower, the count turns green again and the summary loses the red outline.

If you click the "submit" button while the summary is longer than 255 characters, an error notification shows up immediately and no attempt to file the bug is made.

Comment 25

[Ed Morley [:emorley]]

Comment on attachment 8779199 [details] [review]
[treeherder] KWierso:1278746skipversions > mozilla:master

I've left some comments on this PR. 

This is quite an unusual way of implementing this, which I find pretty hard to read (and I think breaks if all the `is_active`s aren't at the start of the array).

Using lodash's findLast(), the 5 lines of code here can be replaced with just one, that is much more readable :-)

Comment 26

[Ed Morley [:emorley]]

Comment on attachment 8779201 [details] [review]
[treeherder] KWierso:1278746longsummaries > mozilla:master

Looks good, thank you! :-)

Comment 27

[Ed Morley [:emorley]]

Comment on attachment 8779200 [details] [review]
[treeherder] KWierso:1278746optionalintermittent > mozilla:master

Also awesome! :-)

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://github.com/mozilla/treeherder/commit/8c6be586aac942cd55d81ad296258d2fc91538d2 landed for the bug summary length PR. I must not have re-added the bug number before rebasing it on its own.

Comment 29

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/721d2f522196a13da31dad06a454802249af63d7
Bug 1278746 - Make 'intermittent-failure' keyword optional (#1772) r=emorley

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8779199 [details] [review]
[treeherder] KWierso:1278746skipversions > mozilla:master

How about like this?

Comment 31

[Ed Morley [:emorley]]

Comment on attachment 8779199 [details] [review]
[treeherder] KWierso:1278746skipversions > mozilla:master

Perfect :-)

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Ed Morley [:emorley] from comment #31)
> Comment on attachment 8779199 [details] [review]
> [treeherder] KWierso:1278746skipversions > mozilla:master
> 
> Perfect :-)

This landed in https://github.com/mozilla/treeherder/commit/ef7dd201732f4f3f4bc2f3430a679ec21206a63b
Dunno why it didn't autocomment.

Comment 33

[Ed Morley [:emorley]]

The commit message didn't contain a bug number.

Comment 34

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8769022 [details] [review]
[treeherder] KWierso:1278746 > mozilla:master

This is for the see_also, depends_on, and blocks fields. Tested this locally against bugzilla-dev and everything appears to work correctly in the cases where I specify any zero, one, two or three of the fields.

Comment 35

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/8ef82f44dc98e1c6de435785eb90214fdbf05beb
Bug 1278746 - Add the ability to set block,depends on,see also fields (#1662) r=emorley

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Whoops, there's still one more PR hanging in this bug before I can close this...

Comment 37

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8771162 [details] [review]
[treeherder] KWierso:showdxrlinks > mozilla:master

Finally getting back to this one. :)


I've changed it so it will always show at least a link to m-c's DXR search results, assuming we can find a filename.
 
If we're on a release branch, we'll show links to DXR results up to and including the current tree. (Beta will show m-c, aurora, beta. Aurora will show m-c and aurora. esr45 will show everything. Etc.)

I've put in an arbitrary limit of 128 characters on what we'll consider a valid potential filename. This should help make sure we don't show dxr links for a failure like in cases where we couldn't really find a file name, but the process for finding a filename gave us a huge string of command parameters or something.


I'll still need to drop the commit that enables the bug filer in all cases before this can land, but it helps in debugging this PR since I only have to add "bugfiler" to the URL rather than running vagrant and creating a user to sign in as.

Comment 38

[Ed Morley [:emorley]]

Let's call this bug fixed and break out any remaining pieces to new bugs.