Show job dependency on the Test overview page #3287

Amrysliu · 2020-07-28T03:20:20Z

Following the way that used in tests page:

use a little branch icon to show the related jobs
different color of the branch icon means different status:
a. when the icon is blue, it means the job has no parent job
b. when the icon is red, it means the job's parent job failed
c. when the icon is green, it means the job's parent job success

See: https://progress.opensuse.org/issues/15850

codecov · 2020-07-28T04:18:59Z

Codecov Report

Merging #3287 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3287      +/-   ##
==========================================
- Coverage   91.50%   91.49%   -0.01%     
==========================================
  Files         215      215              
  Lines       13064    13066       +2     
==========================================
+ Hits        11954    11955       +1     
- Misses       1110     1111       +1

Impacted Files	Coverage Δ
lib/OpenQA/WebAPI/Controller/Test.pm	`96.01% <100.00%> (+0.02%)`	⬆️
lib/OpenQA/WebAPI/Plugin/Helpers.pm	`95.51% <0.00%> (-0.41%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccadcf5...3722ef4. Read the comment docs.

okurz

You reuse and refactor existing code. That is good. But I am not sure why you want to add both the branch and the chain icons at the same time. We should keep the UI well organized and only add distinct elements when they add specific information. Is the branch icon telling me anything more than the chain icon?

t/ui/10-tests_overview.t

Amrysliu · 2020-07-28T07:40:06Z

But I am not sure why you want to add both the branch and the chain icons at the same time. We should keep the UI well organized and only add distinct elements when they add specific information. Is the branch icon telling me anything more than the chain icon?

Because on the tests page, there are two icons related to job's dependency. The branch icon shows that one job has how many children jobs and parents jobs, the chain icon shows parent job's result. I add two icons in order to keep the same with the tests page.

okurz · 2020-07-28T07:53:16Z

Well, on /tests we have separate columns for the job status and job details status. We don't need to replicate all of this on the overview page. I am just worried that this becomes a bit cluttered. We can rethink the design on /tests as well. Let's give others the opportunity to comment as well

Martchus

This makes it at least consistent with the 'All tests' page but I agree with @okurz that it looks a bit cluttered. Maybe you can decrease the opacity and/or size of the buttons to improve that. Some UIs also hide certain elements for a row completely until one hovers above the row. But I'm also not sure whether that makes sense here and what would work best.

assets/javascripts/overview.js

t/ui/10-tests_overview.t

Amrysliu · 2020-07-29T08:08:16Z

@okurz @Martchus Thanks for your suggestions. I modified the code to only show one icon. I use the color of branch icon to tell the state of dependency job. Here is the example:

The blue circle branch icon means there is no parent job
The green circle branch icon means the parent job passed.
The red circle branch icon means the parent job failed.
Any feedback is appreciate. Thanks.

assets/javascripts/overview.js

okurz · 2020-07-29T10:23:50Z

@schlad

The icon on the side could be already useful, but could we also work towards getting some better rendering of parallel tests please? So that all hpc_ALPHA_mpich_mpi_xxx tests, which are in fact '1 multi machine' test, could be easily seen as such? For instance the parent rendered with bold, or children rendered with italic or whatever is the easiest.

Oli knows specifically about this idea; he even prepared mock-up where children were 'collapsible'; I don't like this, as this goes into even more 'nesting' and more needs of 'clicking'; I would favor simple plain view as described above.

That's a good idea for later which I have recorded in https://progress.opensuse.org/issues/15850#note-10 now

okurz

@Amrysliu please put a bit more details in the git commit message describing your changes in a bit more detail.

alvarocarvajald · 2020-07-29T11:39:12Z

This is a very nice improvement!

Just a question: I'm assuming from the code that just like in the /tests/ view, when you hover over the branch icon on a child job, its parent will also be highlighted, and when hovering on the branch icon in the parent, its child(s) will be highlighted, right? This would be very helpful for these views, and could take us closer to rendering parallel jobs somehow grouped as they're in essence just one test.

Thanks!

Martchus · 2020-07-29T11:48:30Z

I don't think the hovering is included in this PR. (@Amrysliu Correct me if I'm wrong.)

Amrysliu · 2020-07-30T03:00:44Z

Just a question: I'm assuming from the code that just like in the /tests/ view, when you hover over the branch icon on a child job, its parent will also be highlighted, and when hovering on the branch icon in the parent, its child(s) will be highlighted, right? This would be very helpful for these views, and could take us closer to rendering parallel jobs somehow grouped as they're in essence just one test.

@Martchus is right. The hovering is not included in this PR.

templates/webapi/test/tr_job_result.html.ep

Amrysliu · 2020-07-30T07:12:41Z

@schlad @alvarocarvajald Thanks for your comments. We have added those ideas to the related ticket and will re-evaluate what need to do next step. Your feedback is appreciated. Thanks again.

assets/javascripts/tests.js

1. use a little branch icon to show the related jobs 2. different color of the branch icon means different status: a. when the icon is blue, it means the job has no parent job b. when the icon is red, it means the job's parent job failed c. when the icon is green, it means the job's parent job success See: https://progress.opensuse.org/issues/15850

okurz · 2020-07-30T11:17:32Z

assets/javascripts/tests.js

-                highlightJobsHtml(childrenToHighlight, parentsToHighlight) +
+        var dependencyResult = showJobDependency(deps);
+        var dependencyHtml = '';
+        if (dependencyResult['title'] !== undefined) {


I think this should also be dependencyResult.title, isn't it?

I guess it isn't that important.

This is a followup for os-autoinst#3287. 1. when putting the mouse over the branch icon: a. highlight parent jobs with red color b. highlight child jobs with blue color 2. Fix a javascript issue Related: os-autoinst#3287 (comment)

This comment has been minimized.

Sign in to view

Amrysliu mentioned this pull request Jul 28, 2020

[WIP] Show job dependency on the tests overview page #3282

Closed

Amrysliu force-pushed the show_dependency_in_overview branch from bdc01b8 to 19757ee Compare July 28, 2020 03:52

okurz requested changes Jul 28, 2020

View reviewed changes

t/ui/10-tests_overview.t Show resolved Hide resolved

Martchus requested changes Jul 28, 2020

View reviewed changes

assets/javascripts/overview.js Outdated Show resolved Hide resolved

Amrysliu force-pushed the show_dependency_in_overview branch from 19757ee to 2874814 Compare July 29, 2020 07:49

okurz reviewed Jul 29, 2020

View reviewed changes

t/ui/10-tests_overview.t Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Amrysliu force-pushed the show_dependency_in_overview branch from 2874814 to 4eb5722 Compare July 29, 2020 08:45

This comment has been minimized.

Sign in to view

Martchus reviewed Jul 29, 2020

View reviewed changes

assets/javascripts/overview.js Outdated Show resolved Hide resolved

okurz reviewed Jul 29, 2020

View reviewed changes

Amrysliu force-pushed the show_dependency_in_overview branch from 4eb5722 to 0326a0e Compare July 30, 2020 06:57

okurz reviewed Jul 30, 2020

View reviewed changes

templates/webapi/test/tr_job_result.html.ep Show resolved Hide resolved

okurz requested changes Jul 30, 2020

View reviewed changes

assets/javascripts/tests.js Outdated Show resolved Hide resolved

Amrysliu force-pushed the show_dependency_in_overview branch from 0326a0e to 3722ef4 Compare July 30, 2020 07:47

okurz approved these changes Jul 30, 2020

View reviewed changes

Martchus approved these changes Jul 30, 2020

View reviewed changes

mergify bot merged commit 090cd4d into os-autoinst:master Jul 30, 2020

Amrysliu deleted the show_dependency_in_overview branch July 31, 2020 02:06

Amrysliu mentioned this pull request Aug 6, 2020

Highlight dependency jobs in the overview page #3297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show job dependency on the Test overview page #3287

Show job dependency on the Test overview page #3287

Amrysliu commented Jul 28, 2020 •

edited

Loading

This comment has been minimized.

codecov bot commented Jul 28, 2020 •

edited

Loading

okurz left a comment

Amrysliu commented Jul 28, 2020 •

edited

Loading

okurz commented Jul 28, 2020

Martchus left a comment

Amrysliu commented Jul 29, 2020

This comment has been minimized.

This comment has been minimized.

okurz commented Jul 29, 2020

okurz left a comment

alvarocarvajald commented Jul 29, 2020

Martchus commented Jul 29, 2020

Amrysliu commented Jul 30, 2020

Amrysliu commented Jul 30, 2020

okurz Jul 30, 2020

Martchus Jul 30, 2020

Show job dependency on the Test overview page #3287

Show job dependency on the Test overview page #3287

Conversation

Amrysliu commented Jul 28, 2020 • edited Loading

This comment has been minimized.

codecov bot commented Jul 28, 2020 • edited Loading

Codecov Report

okurz left a comment

Choose a reason for hiding this comment

Amrysliu commented Jul 28, 2020 • edited Loading

okurz commented Jul 28, 2020

Martchus left a comment

Choose a reason for hiding this comment

Amrysliu commented Jul 29, 2020

This comment has been minimized.

This comment has been minimized.

okurz commented Jul 29, 2020

okurz left a comment

Choose a reason for hiding this comment

alvarocarvajald commented Jul 29, 2020

Martchus commented Jul 29, 2020

Amrysliu commented Jul 30, 2020

Amrysliu commented Jul 30, 2020

okurz Jul 30, 2020

Choose a reason for hiding this comment

Martchus Jul 30, 2020

Choose a reason for hiding this comment

Amrysliu commented Jul 28, 2020 •

edited

Loading

codecov bot commented Jul 28, 2020 •

edited

Loading

Amrysliu commented Jul 28, 2020 •

edited

Loading