Skip to content

Deduplicate openqa-cli options and help #6316

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

perlpunk
Copy link
Contributor

@perlpunk perlpunk commented Mar 23, 2025

Use a spec file to deduplicate the options and help output for openqa-cli.

Advantages

Only one place, no duplicated information that can diverge over time

Currently the global options are repeated in several places:

And the options for the subcommands are also duplicated:

Some of the options were forgotten in the pod, e.g. archive does not list --pretty, --quiet and --verbose

Easy shell completion

@b10n1k had the nice idea to add shell completion for openqa-cli (#6063).

However, manually written completion scripts have several disadvantages. Typos, and the content can diverge over time. Duplicated information again.

With the spec file we can get bas/zsh completion practically for free with appspec, including completion descriptions for bash.
We only have to regenerate it when an option gets added/changed, and a test can ensure that we don't forget regeneration.
So appspec would only be a dependency for us openQA authors.
(Note that this PR doesn't include completion yet)

Copy link

codecov bot commented Mar 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.11%. Comparing base (921e74b) to head (47c3c7b).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6316   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files         399      399           
  Lines       40721    40771   +50     
=======================================
+ Hits        40362    40412   +50     
  Misses        359      359           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sub _print_options ($label, $options) {
my $help = "$label\n";
my @rows;
my $max = 15;
Copy link
Member

Choose a reason for hiding this comment

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

what's the meaning of this magic number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could set it to zero.
It's the width of the left column, determined by the maximum item length, and I just thought it would be nice to have a minimum width if there are only very short items.

Copy link
Member

Choose a reason for hiding this comment

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

ok. how about a constant called accordingly?

Copy link
Contributor Author

@perlpunk perlpunk Mar 26, 2025

Choose a reason for hiding this comment

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

I found a better way. I just use Getopt::Long::Descriptive which provides a function to generate the usage table.
The format of the option spec is compatible between Getopt::Long, Getopt::Long::Descriptive and App::Spec.

@perlpunk perlpunk force-pushed the openqa-cli-spec branch 3 times, most recently from c5a704a to bdd9f0c Compare March 26, 2025 14:50
@perlpunk
Copy link
Contributor Author

Not sure how to solve this error:
https://build.opensuse.org/package/live_build_log/devel:openQA:GitHub:os-autoinst:openQA:PR-6316/openQA:openQA-client-test/openSUSE_Tumbleweed/x86_64

Could not open '/usr/share/openqa/script/../public/openqa-cli.yaml' for reading: No such file or directory

Where should I put a file that the client needs to read?

@kalikiana
Copy link
Member

Not sure how to solve this error: https://build.opensuse.org/package/live_build_log/devel:openQA:GitHub:os-autoinst:openQA:PR-6316/openQA:openQA-client-test/openSUSE_Tumbleweed/x86_64

Could not open '/usr/share/openqa/script/../public/openqa-cli.yaml' for reading: No such file or directory

Where should I put a file that the client needs to read?

How about datadir/openqa/client

@b10n1k
Copy link
Contributor

b10n1k commented Apr 1, 2025

Not sure how to solve this error: https://build.opensuse.org/package/live_build_log/devel:openQA:GitHub:os-autoinst:openQA:PR-6316/openQA:openQA-client-test/openSUSE_Tumbleweed/x86_64

Could not open '/usr/share/openqa/script/../public/openqa-cli.yaml' for reading: No such file or directory

Where should I put a file that the client needs to read?

I guess the easiest would be somewhere in lib/openQA as under /usr/share/openqa there are only lib and script dirs.. even so public seems it should be able to be found in /usr/share/openqa/!! Did you try to build and debug in the build env? Maybe it is about copy the file in that location

This comment was marked as resolved.

@perlpunk perlpunk force-pushed the openqa-cli-spec branch 4 times, most recently from 4d22819 to edaea44 Compare July 20, 2025 15:21
@perlpunk
Copy link
Contributor Author

The file will now be installed to /usr/share/openqa/client/openqa-cli.yaml. In git it's still under public/ for now. That's why the code has to check for both locations.
Alternatively we could create a client/ folder in git.

@perlpunk perlpunk marked this pull request as ready for review July 20, 2025 16:11
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.

Looks good I guess. I only have a small nitpick on the use lines.

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.

5 participants