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

Add openQA status badges #5022

Merged
merged 2 commits into from
Mar 8, 2023
Merged

Add openQA status badges #5022

merged 2 commits into from
Mar 8, 2023

Conversation

asdil12
Copy link
Member

@asdil12 asdil12 commented Feb 23, 2023

Ticket: https://progress.opensuse.org/issues/124173

The badges can be accessed for any job via:

/tests/123/badge
/tests/latest/badge

They support the get parameter show_build=1 that will
prefix the status with the build number (useful for the /latest/badge).

The badge will mostly use the colors as defined in the openQA theme but there are some small changes to ensure readability with the white text.

The width of the badge is automatically aligned to the length of the status text.

3

@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #5022 (659f9c8) into master (f74fdc6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5022   +/-   ##
=======================================
  Coverage   98.19%   98.19%           
=======================================
  Files         379      379           
  Lines       35570    35598   +28     
=======================================
+ Hits        34929    34957   +28     
  Misses        641      641           
Impacted Files Coverage Δ
lib/OpenQA/Schema/Result/Jobs.pm 98.87% <100.00%> (+<0.01%) ⬆️
lib/OpenQA/WebAPI.pm 97.18% <100.00%> (+0.01%) ⬆️
lib/OpenQA/WebAPI/Controller/Test.pm 95.32% <100.00%> (+0.20%) ⬆️
t/ui/18-tests-details.t 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@asdil12
Copy link
Member Author

asdil12 commented Feb 24, 2023

blocked
incomplete
passed
running
scheduled
softfailed
timeout_exceeded
uploading
user_cancelled

Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

A unit test is missing. Note that functions of https://docs.mojolicious.org/Test/Mojo (like element_exists) should also work with SVG.

lib/OpenQA/WebAPI/Controller/Test.pm Outdated Show resolved Hide resolved
lib/OpenQA/WebAPI/Controller/Test.pm Show resolved Hide resolved
templates/webapi/test/badge.svg.ep Outdated Show resolved Hide resolved
templates/webapi/test/badge.svg.ep Outdated Show resolved Hide resolved
templates/webapi/test/badge.svg.ep Outdated Show resolved Hide resolved
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

My previous remarks are all resolved, except the use of base64. Otherwise I have only a few suggestions regarding coding style.

lib/OpenQA/WebAPI/Controller/Test.pm Outdated Show resolved Hide resolved
lib/OpenQA/WebAPI/Controller/Test.pm Outdated Show resolved Hide resolved
lib/OpenQA/WebAPI/Controller/Test.pm Outdated Show resolved Hide resolved
@Martchus
Copy link
Contributor

This isn't merged automatically because the patch coverage is not 100 %. You're very close so I suppose it makes sense to extend the unit tests just a little bit further.

Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

I suggest to carefully check the existing uses of the word "badge". For example there is http://open.qa/docs/#_review_badges from which we should distinguish. I think the route names are fine but in the implementation and whenever a text is visible to users we should use a unique string. How about "test result badge"? But also it looks like you mix states and results.

To be able to help better I would need more explanation about the use cases you want to accomplish.

Please also extend documentation to mention the use. It can be simple, basically what you put into the commit message should be documentation and the commit message should explain your decisions and motivation and design choices.

lib/OpenQA/WebAPI/Controller/Test.pm Outdated Show resolved Hide resolved
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

One inline comment where maybe others have a good idea. And please look into my previous review comment, e.g. about documentation

lib/OpenQA/WebAPI/Controller/Test.pm Show resolved Hide resolved
@asdil12
Copy link
Member Author

asdil12 commented Mar 2, 2023

And please look into my previous review comment, e.g. about documentation

I understood your comment about documentation in the way that you wanted to have a more extensive commit message.
So I added the line:

The badges can be used to show the current status of an openQA job eg. within a github PR.

Is this not what you meant?

@Martchus
Copy link
Contributor

Martchus commented Mar 2, 2023

@asdil12 No, he wants information like what you already had in the commit message to be added in addition to the documentation.

Ticket: https://progress.opensuse.org/issues/124173

The badges can be used to show the current status of an openQA job
eg. within a github PR.

The badges can be accessed for any job via:

/tests/123/badge
/tests/latest/badge

They support the get parameter 'show_build=1' that will
prefix the status with the build number (useful for the /latest/badge).
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

Great PR! Please pay attention to the following items before merging:

Files matching docs/*.asciidoc:

  • Consider generating documentation locally to verify it is rendered correctly using tools/generate-htmldoc

This is an automatically generated QA checklist based on modified files.

docs/UsersGuide.asciidoc Outdated Show resolved Hide resolved
Co-authored-by: Martchus <martchus@gmx.net>
@asdil12 asdil12 requested a review from okurz March 3, 2023 15:35
@mergify mergify bot merged commit df25e2e into os-autoinst:master Mar 8, 2023
foursixnine added a commit to os-autoinst/os-autoinst-distri-opensuse that referenced this pull request Mar 8, 2023
Since os-autoinst/openQA#5022 enables badges on o3, we can now add that to multiple places, not only to showcase but also as a visual aid, especially on PR.
rfan1 pushed a commit to rfan1/os-autoinst-distri-opensuse that referenced this pull request Mar 14, 2023
Since os-autoinst/openQA#5022 enables badges on o3, we can now add that to multiple places, not only to showcase but also as a visual aid, especially on PR.
tblume pushed a commit to tblume/os-autoinst-distri-opensuse that referenced this pull request Mar 30, 2023
Since os-autoinst/openQA#5022 enables badges on o3, we can now add that to multiple places, not only to showcase but also as a visual aid, especially on PR.
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.

4 participants