Bug 122180

Summary: Please display information about pending runs in build.webkit.org/dashboard
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Enhancement CC: pmolnar.u-szeged, thorton, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 122191    
Attachments:
Description Flags
Proposed patch
ap: review-, ap: commit-queue-
proposed patch
timothy: review+
screenshot none

Description Alexey Proskuryakov 2013-10-01 14:31:32 PDT
It would be helpful to have a popover with pending run information. E.g. if I'm seeing failing tests in the last run, I'd like to quickly look up whether there is a fix already in the queue.

Not quite sure what to display - maybe a ChangeLog trimmed up to the first line with an asterisk?
Comment 1 Alexey Proskuryakov 2013-10-01 14:32:57 PDT
(well, that would be a concatenation of ChangeLogs, as there are often multiple in a run).
Comment 2 Timothy Hatcher 2013-10-01 14:35:16 PDT
I think the JSON contains this — an array of the changes in the run.
Comment 3 Alexey Proskuryakov 2013-10-03 13:01:32 PDT
It would also help to have a link to current build buildbot page. Sometimes I wonder why things are taking too long, and it could be that bots are unhealthy.
Comment 4 Tim Horton 2013-10-07 17:20:45 PDT
(In reply to comment #0)
> It would be helpful to have a popover with pending run information. E.g. if I'm seeing failing tests in the last run, I'd like to quickly look up whether there is a fix already in the queue.

+1, this would be fantastically useful
Comment 5 Peter Molnar 2013-11-08 06:26:01 PST
I started to work on this, but the term "pending run" is quite confusing, it seems this term has different meaning on this dashboard page and the Buildbot.

As far as I know, Buildbot has "pending build request", which is a revision that needs to be processed, but it's not started yet. It also has "current build", that is an actually started, but still running build. Most builders could only have 0 or 1 running builds, those who has multiple buildslaves, can have more running builds.

The dashboard displays "pending build" or "pending test run", but it's in fact not Buildbot's "pending build request", but it's the number of currently running builds/tests. 

Which one would you like to see detailed on this Dashboard page?
I assume you'd like to see both lists (running and waiting jobs), as it would make more sense (example scenario: builder tree is red, 2 patches are currently running, but there's 3 more waiting for processing, and one of them is a buildfix for the current build breakage).

So, what should we do?

Option A: to have two separate lists, one for waiting tasks ("pending build request" as Buildbot calls it), and a second list for currently running builds.

Option B: to merge those lists together some way, and also sum up the numbers of the waiting and running builds.
Comment 6 Alexey Proskuryakov 2013-11-08 09:53:30 PST
I previously posted some comments about this to bug 122191.

The question to ask here is what information a person using this tools needs to have immediately available. We should provide it, but at the same time it's critically important to keep the view uncluttered, keeping the tool immediately useful without any training.

I think that there are two aspects:

- When there are failing tests, it's very useful to know whether a fix has already been committed. So, there should be a quick way to learn about all commits made after the latest test run (more precisely, a run with the highest revision number). And that's not even limited to commits the buildbot already knows about.

A popover with appropriately shortened ChangeLogs will hopefully work for this purpose.

- When bots are not feeling well, and not processing builds/tests quickly, bot watchers should know about that too. Perhaps there could be an indication of how long the latest run is taking, which could even turn red if that's way more than previous runs. Or maybe it should turn red when there is a backlog of revisions to build/test, but the bot is idle.

Buildbot's ideas about pending runs are not particularly relevant for either aspect.
Comment 7 Peter Molnar 2013-11-12 04:19:16 PST
Created attachment 216653 [details]
Proposed patch

> - When there are failing tests, it's very useful to know whether a fix has already been committed. So, there should be a quick way to learn about all commits made after the latest test run (more precisely, a > run with the highest revision number). And that's not even limited to commits the buildbot already knows about.
> 
> A popover with appropriately shortened ChangeLogs will hopefully work for this purpose.

Attached a patch, what do you think about this kind of display? Is this suitable for your needs?

> - When bots are not feeling well, and not processing builds/tests quickly, bot watchers should know about that too. Perhaps there could be an indication of how long the latest run is taking, which could even > turn red if that's way more than previous runs. Or maybe it should turn red when there is a backlog of revisions to build/test, but the bot is idle.

I think this should be done in a separate bug, I'm planning to do this soon.
Comment 8 Alexey Proskuryakov 2013-11-12 09:15:18 PST
I applied the patch locally, and couldn't get it to work. Hovering "(2) pending test runs" doesn't show any pop-up.

A couple comments though:

1. We should get information about newer revisions from trac, not from buildbot's builder. Even if buildbot hasn't built a binary for the revision yet, it's still relevant for the purpose of knowing what else landed after the latest test run.

2. It would be better to use a real pop-up, not a title. One may want to select and copy some text, there could be rich styling, and possibly even interactive elements in the future. Tim previously suggested reusing Web Inspector code.
Comment 9 Peter Molnar 2013-11-12 11:47:53 PST
(In reply to comment #8)
> I applied the patch locally, and couldn't get it to work. Hovering "(2) pending test runs" doesn't show any pop-up.

The hover is on the revision link of the finished test runs. It displays the revision range between that test run, and the latest revision the builder of the same platform knows about. 

> A couple comments though:
> 
> 1. We should get information about newer revisions from trac, not from buildbot's builder. Even if buildbot hasn't built a binary for the revision yet, it's still relevant for the purpose of knowing what else landed after the latest test run.

Is there a significant delay here, that we have to deal with?


> 2. It would be better to use a real pop-up, not a title. One may want to select and copy some text, there could be rich styling, and possibly even interactive elements in the future. Tim previously suggested reusing Web Inspector code.

I'll check Web Inspector code.
Comment 10 Alexey Proskuryakov 2013-11-12 12:18:28 PST
> The hover is on the revision link of the finished test runs. 

This works indeed. I don't think that this is a place where people would look for it, I certainly couldn't find it myself.

I think that in addition to bug title, displaying author name/nick would be useful, if there is enough space for that.

> Is there a significant delay here, that we have to deal with?

Yes - it's been many times that I yelled at people about breakage that they fixed 5 minutes ago.
Comment 12 Peter Gal 2013-11-21 09:01:25 PST
Comment on attachment 216653 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216653&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:83
> +                linkElement.title= title;

Missing whitespace before the '=' sign.
Comment 13 Alexey Proskuryakov 2013-12-25 22:57:27 PST
Comment on attachment 216653 [details]
Proposed patch

I'm adding code to load SVN revisions from Trac in bug 122191, so we should use this data for display.
Comment 14 Alexey Proskuryakov 2013-12-29 02:04:35 PST
Created attachment 220081 [details]
proposed patch
Comment 15 Alexey Proskuryakov 2013-12-29 02:05:46 PST
Created attachment 220082 [details]
screenshot
Comment 16 Alexey Proskuryakov 2013-12-29 02:20:04 PST
Comment on attachment 220081 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=220081&action=review

> Tools/ChangeLog:60
> +        Like JS counterpart, mostly lifted from Web Inspector.

Oh, also added z-index:1000, because Dashboard has an element with non-zero z-index.
Comment 17 Timothy Hatcher 2013-12-29 08:17:44 PST
Comment on attachment 220081 [details]
proposed patch

Looks good to me. Nice work!
Comment 18 Alexey Proskuryakov 2013-12-29 09:21:49 PST
Committed <http://trac.webkit.org/r161120>.