Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show "failed modules" with a coloring resembling an failed state #4475

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

okurz
Copy link
Member

@okurz okurz commented Jan 24, 2022

Especially with the new openQA feature of "force_result" it is more
common to see openQA jobs where the result is passed or soft-failed
while there are still failed modules. Right now e.g. in /tests/overview
"failed modules" show up on a green background which could confuse
people thinking of these modules as not failed. We should reconsider
the coloring. This commit changes the styling so that the failed modules
in openQA are rendered with a slightly more alarming design using a
light red background and red border which I made a bit thicker.

Before:
Screenshot_20220124_125343_before_failed_modules_green_border

After:
Screenshot_20220124_125607_after_failed_modules_red_border

Related progress issue: https://progress.opensuse.org/issues/95479

Especially with the new openQA feature of "force_result" it is more
common to see openQA jobs where the result is passed or soft-failed
while there are still failed modules. Right now e.g. in /tests/overview
"failed modules" show up on a green background which could confuse
people thinking of these modules as not failed. We should reconsider
the coloring. This commit changes the styling so that the failed modules
in openQA are rendered with a slightly more alarming design using a
light red background and red border which I made a bit thicker.

Related progress issue: https://progress.opensuse.org/issues/95479
@okurz okurz marked this pull request as draft January 24, 2022 13:52
@okurz okurz marked this pull request as ready for review January 24, 2022 13:53
@okurz
Copy link
Member Author

okurz commented Jan 24, 2022

Adding label "not-ready" to give a chance to more people to provide their feedback.

Copy link
Contributor

@dcermak dcermak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to make the border less prominent. Right now it looks like these nodes are selected due to the outline.

@okurz
Copy link
Member Author

okurz commented Jan 27, 2022

I would suggest to make the border less prominent. Right now it looks like these nodes are selected due to the outline.

Valid point. How about the middle ground of 2px or do we need to change the color or style differently?

@jknphy
Copy link
Contributor

jknphy commented Jan 27, 2022

my 2 cents, for soft-failures I would prefer keep the background yellow, it might be a bit distracting. For job groups with few test suites could be useful, but for job groups with huge number of test suite is not something you are going to check when you review it, it is better to trust in the automation that post those failures and group them, so visually doesn't help in those cases to the reviewer.

@okurz
Copy link
Member Author

okurz commented Jan 27, 2022

my 2 cents, for soft-failures I would prefer keep the background yellow, it might be a bit distracting.

The problem is that the background for failed modules was never yellow but green so viewers can be confused about "green == failed"

For job groups with few test suites could be useful, but for job groups with huge number of test suite is not something you are going to check when you review it, it is better to trust in the automation that post those failures and group them, so visually doesn't help in those cases to the reviewer.

I agree. There it shouldn't make a difference.

@jknphy
Copy link
Contributor

jknphy commented Jan 27, 2022

the background for failed modules was never yellow but green so viewers can be confused about "green == failed"

Still I think it is distracting that the soft-failure shows with red background, I would prefer it to see it green (or any neutral color) than red in this PR (but it is just my opinion), and perhaps update the title to reflect both intentions, those are not 'failed' modules.

@dcermak
Copy link
Contributor

dcermak commented Jan 27, 2022

I would suggest to make the border less prominent. Right now it looks like these nodes are selected due to the outline.

Valid point. How about the middle ground of 2px or do we need to change the color or style differently?

Honestly, I would consult someone from the design team about this.

@okurz
Copy link
Member Author

okurz commented Jan 27, 2022

Honestly, I would consult someone from the design team about this.

I would like that :) Can we ping someone over github easily?

@dcermak
Copy link
Contributor

dcermak commented Jan 27, 2022

Honestly, I would consult someone from the design team about this.

I would like that :) Can we ping someone over github easily?

cc @cyntss ☝️

@okurz
Copy link
Member Author

okurz commented Feb 3, 2022

No response within a week. That's fine. I would like to go ahead. After all we can still easily tweak it afterwards. Any feedback afterwards is still appreciated.

@okurz okurz removed the not-ready label Feb 3, 2022
@mergify mergify bot merged commit 2a0ae79 into os-autoinst:master Feb 3, 2022
@okurz okurz deleted the feature/failed_module_recoloring branch February 3, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants