1192917 - Update 'unknown' steps and jobs to a more informative UX

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P3)

Description

[Jonathan French (:jfrench)]

Currently unknown jobs in Treeherder are black, and unknown steps in Logviewer have no color.

So lets update them both to a consistent color, and a more intuitive one than black. I posed a bootstrap-esque 'warning' yellow to Ed in channel and it seemed well received.

Initial proposed:
color: 138, 109, 59
background: 252, 248, 227
border-color: 251, 216, 144

...and perhaps some extra "(unknown step)" text adjacent to the step name in Logviewer for visibility and a11y.

Here is an example of an unknown and unstyled step, "Running tests"
https://treeherder.allizom.org/logviewer.html#?job_id=9386596&repo=mozilla-inbound

Comment 1

[Jonathan French (:jfrench)]

See bootstrap also for the reference point on the proposed appearance
http://getbootstrap.com/components/#alerts

Comment 2

[Jonathan French (:jfrench)]

Attachment: unknownStepLogviewer

This is what I'd like to go with for an unknown logviewer step, to raise visibility both when the step is selected (per screen grab), and when it isn't.

Comment 3

[Jonathan French (:jfrench)]

Attachment: PR 859

Please see above PR for status and review.

Comment 4

[Jonathan French (:jfrench)]

Ed, do you have any preferences for a color for coalesced? (currently a mustard-ish yellow). Since we're planning to use a different bootstrap yellow here for unknown.

If not I'll try to find a suitable hue :)

Comment 5

[Jonathan French (:jfrench)]

Oh, and you can check/compare if needed in the PR conversation.

Comment 6

[Ed Morley [:emorley]]

Happy for you to pick - you have a good eye for this :-)

Comment 7

[Cameron Dawson [:camd]]

Comment on attachment 8646438 [details] [review]
PR 859

Clearing the review.  Things look good code-wise.  But it'll need to be rebased and I think there's a color decision to make on coalesced jobs.  Please reassign when you've got it ready.  Looks great!

Comment 8

[Jonathan French (:jfrench)]

Comment on attachment 8646438 [details] [review]
PR 859

Ok I think I'm ready for another review. That was a bit of rebasing over the job counts work :)

:camd if you want to locally check the yellow unknown and light blue coalesced buttons you can switch say running btn-dkgray with btn-yellow or btn-ltblue in services/providers.js. Since both are not that common for testing against counts or large numbers of jobs.

You can also look at coalesced here in my gh-pages at the moment

http://tojonmz.github.io/treeherder/ui/#/jobs?repo=mozilla-inbound&revision=823aa80c5ffa&filter-resultStatus=success&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=coalesced

Comment 9

[Jonathan French (:jfrench)]

Comment on attachment 8646438 [details] [review]
PR 859

More rebasing work to do so removing the r?. The Logviewer contents need tweaking.

Comment 10

[Cameron Dawson [:camd]]

Comment on attachment 8646438 [details] [review]
PR 859

Looks great!  Thanks for doing this!

Comment 11

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/e644334fa4fa5969da90c83dacc2f2408527c37e
Bug 1192917 - Handle 'unknown' steps and jobs with improved UI

Comment 12

[Jonathan French (:jfrench)]

Marking fixed per above merge.

Comment 13

[Ed Morley [:emorley]]

A push has been made to stage with a grunt build, so we can see what this looks like on stage now:
https://treeherder.allizom.org/logviewer.html#?job_id=10246382&repo=mozilla-inbound

I think since a result of "unknown" is expected for the placeholder steps we create for taskcluster jobs, the warning icons/text on each unknown step appears on all taskcluster jobs. Perhaps we should remove the warning icon/text and just keep the other style changes - otherwise it looks like something has broken, when in fact it's expected?

Comment 14

[Ed Morley [:emorley]]

s/I think since/Since/

Also, this needn't block the next prod push, let's just tweak this in another bug at some point :-)

Comment 15

[Jonathan French (:jfrench)]

(In reply to Ed Morley [:emorley] from comment #13)
> I think since a result of "unknown" is expected for the placeholder steps we
> create for taskcluster jobs, the warning icons/text on each unknown step
> appears on all taskcluster jobs. Perhaps we should remove the warning
> icon/text and just keep the other style changes - otherwise it looks like
> something has broken, when in fact it's expected?

Ya I noticed this change while this PR was open and during which time that Taskcluster change landed :) Prior to it we had some legitimate cases where some were 'unknown' and others were ''. I thought all those new unknown Taskcluster steps legitimately needed a yellow display state, but I'll remove the step warnings in a separate bug/PR.

Comment 16

[Jonathan French (:jfrench)]

Now bug 1199659.

Comment 17

[Jonathan French (:jfrench)]

Logviewer unknown steps and the Coalesced job button change verified fixed on production. Unknown job buttons were not verified, as we don't have a filter for them at present and none were immediately visible.