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

Cloudregistration test module improvements #20260

Conversation

linguini-dev
Copy link
Contributor

@linguini-dev linguini-dev commented Sep 24, 2024

We have a list of manual steps which are executed to verify cloud-regionsrv-client. The steps can be found here: https://github.com/SUSE-Enceladus/cloud-regionsrv-client/blob/master/integration_test-process.txt

Our existing automated tests did not cover all of the steps listed so this PR aims to remedy that.

This PR:

  • Adds missing steps to the test module.
  • Removes duplicate imports (e.g. use version_utils 'is_sle';)
  • Defines a few new functions. These are used to retain the existing logical flow of the test while adding some extra steps along the way.
  • Swaps piped commands for simpler (imo) options. e.g. ls /etc/zypp/credentials.d/ | wc -l vs ls -A /etc/zypp/credentials.d/.
  • Adds steps to verify container runtimes.
  • Executes Tidy on the code, hence the last empty line.

Additional info:

Verification runs:

@m-dati
Copy link
Contributor

m-dati commented Sep 24, 2024

CI claiming tidy needed

@linguini-dev linguini-dev marked this pull request as ready for review September 25, 2024 10:58
Copy link
Member

@asmorodskyi asmorodskyi left a comment

Choose a reason for hiding this comment

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

LGTM

use version_utils 'is_sle';

my $path = is_sle('=15-SP2') ? '/usr/sbin/' : ''; # 15-SP2 is the oldest version that needs fullpaths
my $regcode_param = (is_byos()) ? "-r " . get_required_var('SCC_REGCODE') : '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had a discussion with @asmorodskyi, that is the intended behavior and has worked like that so far. I am just removing one of the checks as it is impossible due to us not supporting versions that old.

@@ -19,7 +19,9 @@ use strict;
use utils;
use publiccloud::utils;
use publiccloud::ssh_interactive 'select_host_console';
use version_utils 'is_sle';

my $path = is_sle('=15-SP2') ? '/usr/sbin/' : ''; # 15-SP2 is the oldest version that needs fullpaths
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per the comment, # 15-SP2 is the oldest version that needs fullpaths i.e. No other version needs the fullpath, so no need to use it.

@linguini-dev linguini-dev force-pushed the feature/cloudregistration-improvements branch from ce1f098 to 96b5145 Compare October 7, 2024 12:29
tests/publiccloud/check_registercloudguest.pm Outdated Show resolved Hide resolved
tests/publiccloud/check_registercloudguest.pm Outdated Show resolved Hide resolved
tests/publiccloud/check_registercloudguest.pm Outdated Show resolved Hide resolved
tests/publiccloud/check_registercloudguest.pm Outdated Show resolved Hide resolved
@linguini-dev linguini-dev force-pushed the feature/cloudregistration-improvements branch 2 times, most recently from 657d1df to ca9e91c Compare October 8, 2024 07:50
@linguini-dev linguini-dev force-pushed the feature/cloudregistration-improvements branch from ca9e91c to 9c00e63 Compare October 8, 2024 08:40
@asmorodskyi asmorodskyi merged commit a4626b0 into os-autoinst:master Oct 8, 2024
10 checks passed
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.

7 participants