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 usage+help to all scripts #331

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

okurz
Copy link
Member

@okurz okurz commented Jul 23, 2024

  • trigger-pflash-test: Allow to import for testability and extendability
  • trigger-openqa_in_openqa: Allow to import for testability and extendability
  • os-autoinst-testapi-usage: Allow to import for testability and extendability
  • os-autoinst-obs-auto-submit: Allow to import for testability and extendability
  • openqa-schedule-mm-ping-test: Allow to import for testability and extendability
  • openqa-query-for-job-label: Allow to import for testability and extendability
  • openqa-monitor-investigation-candidates: Allow to import for testability and extendability
  • openqa-monitor-incompletes: Allow to import for testability and extendability
  • openqa-label-known-issues-multi: Allow to import for testability and extendability
  • openqa-label-known-issues-hook: Allow to import for testability and extendability
  • openqa-label-known-issues-and-investigate-hook: Allow to import for testability and extendability
  • openqa-label-known-issues: Allow to import for testability and extendability
  • openqa-investigate-multi: Allow to import for testability and extendability
  • openqa-incompletes-stats: Allow to import for testability and extendability
  • openqa-advanced-retrigger-jobs: Allow to import for testability and extendability
  • obs-check-package-origin: Allow to import for testability and extendability
  • obs-check-devel-openqa-leap-extra-dependencies: Allow to import for testability and extendability
  • monitor-openqa_job: Allow to import for testability and extendability
  • trigger-pflash-test: Add usage+help
  • trigger-openqa_in_openqa: Add usage+help
  • reboot-stability-check: Add usage+help
  • os-autoinst-testapi-usage: Add usage+help
  • os-autoinst-obs-auto-submit: Add usage+help
  • openqa-schedule-mm-ping-test: Add usage+help
  • openqa-query-for-job-label: Add usage+help
  • openqa-monitor-investigation-candidates: Add usage+help
  • openqa-monitor-incompletes: Add usage+help
  • openqa-label-known-issues-multi: Add usage+help
  • openqa-label-known-issues-hook: Add usage+help
  • openqa-label-known-issues-and-investigate-hook: Add usage+help
  • openqa-investigate-multi: Add usage+help
  • openqa-investigate: Add usage+help
  • openqa-incompletes-stats: Add usage+help
  • openqa-advanced-retrigger-jobs: Add usage+help
  • obs-check-package-origin: Add usage+help
  • obs-check-devel-openqa-leap-extra-dependencies: Add usage+help
  • monitor-openqa_job: Add usage+help
  • backlog-set-due-date: Add usage+help
  • backlog-check-wip-limit: Add usage+help
  • openqa-label-known-issues: Add argument parsing
  • t: Add test for help of openqa-label-known-issues

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

Copy link
Contributor

mergify bot commented Jul 23, 2024

This pull request is now in conflicts. Could you fix it? 🙏

[[ $job_state = 'done' ]] && break
usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add an example here on how to invoke this script (how to supply job id(s?)).

cat << EOF
Usage: $0 [OPTIONS]

Checks devel:openQA:Leap: OBS project dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Please be a bit more descriptive here.
Even reading the source code I'm still not sure what exactly this script does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, me neither

Copy link
Member

Choose a reason for hiding this comment

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

If nobody knows what this does, we might want to delete the script.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the script is actively used within https://gitlab.suse.de/openqa/scripts-ci/

exit 99
usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Please include positional argument(s) in usage.


usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Please provide an example call and/or a list of arguments (that are supplied via env in this case I guess)


usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Please provide an example call and/or a list of arguments (that are supplied via env in this case I guess)

cat << EOF
Usage: $0 [OPTIONS]

Get statistics about recent incomplete openQA jobs.
Copy link
Member

@asdil12 asdil12 Jul 23, 2024

Choose a reason for hiding this comment

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

What kind of statistics? Does it simply print the sum of jobs that incompleted since $failed_since?

@@ -388,4 +385,29 @@ main() {
investigate "$@"
}

usage() {
cat << EOF
Usage: echo jobnumber | $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Please add some info on host_url, dry_run, verbose, prio_add, …

@@ -18,4 +28,16 @@ main() {
exit $rc
}

usage() {
cat << EOF
Usage: echo jobnumber | $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Same as above


Takes an openQA job URL, looks for matching "known issues", for example from
progress.opensuse.org, labels the job and retriggers if specified in the issue
(see the source code for details how to mark tickets).
Copy link
Member

Choose a reason for hiding this comment

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

Please add some text on what a known issues is and how it is found in progress.
Also maybe add an example on these labels. I'm not even sure if this refers to labels on openQA jobs or on some magic strings in progress ticket subjects.
Looking at the source code I cannot answer this question - it would be more of a prerequisite to even understand the source.

@@ -46,4 +55,18 @@ hook() {
fi
}

usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

Choose a reason for hiding this comment

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

Please update the usage according to the description below


"hook script" intended to be called by openQA instances taking a job ID as
parameter and forwarding a complete job URL to "openqa-label-known-issues" on
stdin and all left unknowns to "openqa-investigate"
Copy link
Member

Choose a reason for hiding this comment

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

What do 'left unknowns' refer to? Is this dependent on the output of openqa-label-known-issues that tells this script if a job is unknown (whatever that means)?

Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

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

I didnt go through all the changes for now. But one thing, maybe a picky one, is to add an expected output.
Also some can be piped. if there are some common cases of those cases, it would be nice to documented somewhere.


usage() {
cat << EOF
Usage: $0 [OPTIONS]
Copy link
Member

@asdil12 asdil12 Jul 23, 2024

Choose a reason for hiding this comment

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

Please update the usage according to the description below.
Maybe add an example

cat << EOF
Usage: $0 [OPTIONS]

Checks the number of tickets in a redmine query against a "WIP-Limit".
Copy link
Member

@asdil12 asdil12 Jul 23, 2024

Choose a reason for hiding this comment

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

Please document at least the most important env vars and what happens if check fails (exit code=1?)

trigger-pflash-test Outdated Show resolved Hide resolved
cat << EOF
Usage: $0 [OPTIONS]

Triggers tests on an openQA instance testing openQA itself.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Triggers tests on an openQA instance testing openQA itself.
Schedules tests on an openQA instance testing openQA itself.
Using the openQA-in-openQA test distribution from:
https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-openQA

Copy link
Member Author

Choose a reason for hiding this comment

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

The application is called "trigger" so I would prefer to reuse that wording

cat << EOF
Usage: $0 [OPTIONS] HOSTS...

Conducts reboot stability checks against a space- separated list of hosts.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Conducts reboot stability checks against a space- separated list of hosts.
Conducts reboot stability checks against a space- separated list of hosts.
Each host is pinged, checked for listening on port 22 and sshed to and rebooted several times.

shift
usage() {
cat << EOF
Usage: $0 path_to_testapi path_to_code_repository [path_to_another_repository] [...]
Copy link
Member

Choose a reason for hiding this comment

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

What does this script do and why?

Copy link
Member Author

Choose a reason for hiding this comment

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

I really don't know

backlog-set-due-date Outdated Show resolved Hide resolved
backlog-set-due-date Outdated Show resolved Hide resolved
reboot-stability-check Outdated Show resolved Hide resolved
test/01-label-known-issues.t Outdated Show resolved Hide resolved
@okurz
Copy link
Member Author

okurz commented Jul 29, 2024

I applied the suggested changes where possible. I will need to incorporate the other comments and rewrap where necessary. No need to review more in the current state.

@okurz okurz force-pushed the feature/usage_and_help branch from daf688b to acf742e Compare July 29, 2024 12:03
-h | --help) usage 0 ;;
-H | --host)
host=$2
shift
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
shift
shift 2

Copy link
Contributor

mergify bot commented Oct 14, 2024

This pull request is now in conflicts. Could you fix it? 🙏

okurz added 29 commits October 18, 2024 09:26
@okurz okurz force-pushed the feature/usage_and_help branch from acf742e to a3722ac Compare October 18, 2024 07:29
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