Bug 51942 - univention-run-diagnostic-checks: Do all checks by default
Summary: univention-run-diagnostic-checks: Do all checks by default
Status: CLOSED FIXED
Alias: None
Product: UCS
Classification: Unclassified
Component: UMC - System diagnostic
Version: UCS 4.4
Hardware: Other Linux
: P5 normal
Target Milestone: UCS 4.4-6-errata
Assignee: Max Pohle
QA Contact: Felix Botner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-04 11:03 CEST by Christian Völker
Modified: 2020-11-04 14:49 CET (History)
4 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 1: Cosmetic issue or missing function but workaround exists
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.011
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2020072021000391
Bug group (optional):
Customer ID: 02302
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Völker univentionstaff 2020-09-04 11:03:05 CEST
In troubleshooting situations it is really annoying when the command line tool
univention-run-diagnostic-checks 
requests the username and password and then complains about missing argument "-t" for tests.

Either set if no "-t" is given a "-t all" as default
or complain about the missing argument before the credentials are asked!
Comment 2 Florian Best univentionstaff 2020-10-20 09:35:57 CEST
REOPEN: git:d04a2f892d37315d4e46bdea01021ccd624ed5a0 introduces a lot of white-space error. Please re-indent with tabs.

Please also add "from six.moves import input as raw_input".
Comment 3 Max Pohle univentionstaff 2020-10-23 09:55:29 CEST
The improvements have been made.

Calling `univention-run-diagnostic-checks` without parameters now causes all tests to be executed. A log-in is required.

A new command line option was implemented, that is `univention-run-diagnostic-checks -t list`. It lists all available tests. This action however requires a log-in. 

The default `--help` does no longer show all available tests, but can now be displayed without the necessarily to log in.

[QA]
fix: 5.0.1-51A~4.4.6.202010201525
Comment 4 Christian Castens univentionstaff 2020-10-23 10:00:10 CEST
Package: univention-management-console-module-diagnostic
Version: 5.0.1-51A~4.4.6.202010201525
Branch: ucs_4.4-0
Scope: errata4.4-6

cases tested:
executing univention-run-diagnostic-checks:
1. without arguments: executes all tests (login necessary).     OK
2. -t <testname>: executes the given test(s) (login necessary). OK
3. -t list: shows list of all tests (login necessary).          OK
4. -t all: executes all tests (login necessary).                OK
5. --help: shows help (no login necessary).                     OK

yaml:        OK
changelog:   OK


please create 5.0-0 merge request
Comment 5 Max Pohle univentionstaff 2020-10-26 12:26:41 CET
Merge to 5.0 was requested:

https://git.knut.univention.de/univention/ucs/-/merge_requests/20
Comment 6 Felix Botner univentionstaff 2020-10-26 12:35:26 CET
"Checks for command line arguments and parameters are done before the user has to enter credentials."

this is missing, just move the args.password and args.username to the end of the parameter checks
Comment 7 Florian Best univentionstaff 2020-10-26 12:46:09 CET
Why don't you add -l/--list instead of now "-t list"?
Comment 8 Max Pohle univentionstaff 2020-10-29 15:52:28 CET
Florian wrote:
> Why don't you add -l/--list instead of now "-t list"?

Hi Florian!

I was considering `-l` against `-t list` and the truth is, that because there was already `-t all` as a special case I considered the decision a matter of taste. But implementing `-l` would have otherwise tempted me to remove `-t all` as well, because that special case is not required any more and now covered by calling the program without parameters.



Hi Felix!

We have discussed the topic in person and came to the conclusion that the current implementation is sufficient. The login details are required in order to display a list with all test names, because this list is then obtained via the univention-management-console. But we can check all other parameters and display the help text without knowing the specific test names and that is how it has been implemented.
Comment 9 Felix Botner univentionstaff 2020-11-02 10:30:48 CET
OK - yaml
OK - univention-management-console-module-diagnostic
OK - merge request