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 system info check #8462
Add system info check #8462
Conversation
tests/console/check_system_info.pm
Outdated
# Summary: Check system info like `/etc/issue` or `/etc/os-release`. | ||
# Maintainer: Alynx Zhou <alynx.zhou@suse.com> | ||
|
||
use base "basetest"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would propose to use consoletest
as parent class, as it has already postfail hooks.
tests/console/check_system_info.pm
Outdated
} | ||
|
||
sub test_flags { | ||
return {fatal => 0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to set this to 0
, as by default no flags are set.
tests/console/check_system_info.pm
Outdated
sub run { | ||
select_console('root-console'); | ||
|
||
script_output("SUSEConnect --list"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not to convert that to assert_script_run
to at least test that exit code is zero. Otherwise we just list some info which is not visible on the dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the output may be out of one screen and using script_output
can print them to serial port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can redirect output in the assert_script_run
too using > /dev/$serialdev
. But my question is more about if we want just print somethingor we want to check the output and have the test for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your advice. Currently we just want to print something, because we don't have source to compare with those output. Those code is providing a way for us to check those things manually while reviewing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, then let's keep that in mind for next steps. Cheers ;)
script_output("SUSEConnect --status-text"); | ||
|
||
if (!get_var('MILESTONE_VERSION')) { | ||
assert_script_run('cat /etc/issue'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar question to this check, as of now we basically just check file existence.
my $milestone_version = get_var('MILESTONE_VERSION'); | ||
assert_script_run("grep $milestone_version /etc/issue"); | ||
} | ||
assert_script_run('cat /etc/os-release'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar thing here, I would propose to check content of the file that it matches version, distribution, etc. This check will fail only if file is missing or cannot be read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwx788 Thanks for your comments, i have created another ticket and try to check the result and avoid manual check.
https://progress.opensuse.org/issues/57242
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar thing here, I would propose to check content of the file that it matches version, distribution, etc. This check will fail only if file is missing or cannot be read.
There's already console/check_os_release
doing that. It's being scheduled in load_extra_console_tests
as well as for SLES4SAP and HA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More info in #7521
Hey! Is it ready now? If so, could you please remove |
I am running test to ensure what I changed can work, but some other modules blocked it so I run it again and again with EXCLUDE_MODULES... |
@@ -1126,6 +1126,7 @@ sub load_consoletests { | |||
loadtest "console/xorg_vt"; | |||
} | |||
loadtest "console/zypper_lr"; | |||
loadtest "console/check_system_info" if (is_sle && (get_var('SCC_ADDONS') !~ /ha/) && !is_sles4sap && (is_upgrade || get_var('MEDIA_UPGRADE'))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be safely scheduled for SLES4SAP as I don't believe we have any test using variable MILESTONE_VERSION
and without it the module only check for files that should exist in SLES4SAP. WDYT @ldevulder
Add check for
/etc/issue
,/etc/os-release
, etc.