action #45008
closed[functional][u] Further improvements on splitting backend code
100%
Description
Things that can be done in the future to further separate backend specific or generic code from other packages...
Suggestions:¶
TODO: all this come from version_utils.pm, does it make sense to move them to Utils::Backends
# is_hyperv
# is_hyperv_in_gui
# is_aarch64_uefi_boot_hdd
# is_svirt_except_s390x
Checks for specific architectures can also be moved to a different package (Utils::Architectures) and opensusebasetest inheriting from it, to avoid excess of imports...
# defined in lib/version_utils.pm
# sub is_s390x {
# return check_var('ARCH', 's390x');
# }
# sub is_x86_64 {
# return check_var('ARCH', 'x86_64');
# }
- Grep for
check_var('ARCH'(might use single or double quotes) and move these to an architecture specific utils package - grep for
'check_var("BACKEND",(might use single or double quotes) and move these to Utils::Backends or a possibly different package to ensure that we can differentiate from utilities that help with the backend, vs utilities that add more functionality to the backend independant code (like is_remote_backend)
Updated by okurz over 5 years ago
- Subject changed from Further improvements on splitting backend code to [functional][u] Further improvements on splitting backend code
- Category set to Enhancement to existing tests
Try to find a category and tags so that we have less work during refinement or triaging: https://progress.opensuse.org/projects/openqatests/wiki#ticket-backlog-refinement . I know, it's just a subtask but we use these easy queries to list the "to be triaged" tickets which shows up this ticket because it did have neither category nor tags. Maybe we can just make that a related ticket and not subtask as otherwise we should finish this one before completing the parent.
Updated by szarate over 5 years ago
- Parent task deleted (
#33388)
Whops! I forgot to add the functional u tag! thanks! Changed the task relationship better.
Updated by szarate over 5 years ago
- Follows action #33388: [functional][u][easy][pvm] Implement proper split from other backends added
Updated by okurz about 5 years ago
- Status changed from New to Feedback
- Assignee set to okurz
Updated by okurz about 5 years ago
PR merged, waiting for feedback regarding breaking any tests.
Updated by okurz about 5 years ago
- Status changed from Feedback to Workable
- Assignee deleted (
okurz)
PR merged, no message received about broken tests. Next steps as mentioned in the suggestion can be followed on with.
Updated by okurz about 5 years ago
- Status changed from Workable to In Progress
- Assignee set to okurz
- Target version changed from Milestone 22 to Milestone 23
Updated by okurz about 5 years ago
- Start date changed from 5000-01-02 to 2019-02-18
due to changes in a related task
Updated by okurz about 5 years ago
- Status changed from In Progress to Feedback
PR merged, let's see if it causes any more issues.
Updated by okurz about 5 years ago
- Status changed from Feedback to Workable
- Assignee deleted (
okurz) - Target version changed from Milestone 23 to Milestone 25
seems like no problems so far. Based on the example of my PR I suggest to also get rid of the other architecture/backend specific bootloader test modules and call them as library functions from the main bootloader as well as extend that approach to other areas of code.
Updated by zluo about 5 years ago
- Status changed from Workable to In Progress
- Assignee set to zluo
take over.
Updated by zluo about 5 years ago
remove following from version_utils.pm to Utils::Backends at first and add use parts for them in main_common.pm, main.pm
# is_hyperv
# is_hyperv_in_gui
# is_aarch64_uefi_boot_hdd
# is_svirt_except_s390x
Updated by zluo about 5 years ago
modified: lib/Utils/Backends.pm
modified: lib/main_common.pm
modified: lib/opensusebasetest.pm
modified: lib/version_utils.pm
modified: products/sle/main.pm
modified: tests/installation/data_integrity.pm
modified: tests/installation/installation_overview.pm
Updated by zluo about 5 years ago
I would create 2 sub tickets because of to many files got changed and this is hard to review and check for merge them later.
Updated by zluo about 5 years ago
- Status changed from In Progress to Workable
working on sub tickets at first.
Updated by zluo almost 5 years ago
- Status changed from Workable to Feedback
2 sub tickets still not done yet, set this ticket for now as feedback
Updated by SLindoMansilla almost 5 years ago
- Status changed from Feedback to In Progress
I would say in progress is better. Since this parent ticket doesn't need review.
Updated by szarate almost 5 years ago
- Status changed from In Progress to Resolved
Backend functions have moved out of lib/utils, however there's still a lot of work to do... But at least there are stepping stones to unclutter lib/utils.pm (Let's just make sure that whatever is moved, is used... as of now, there are some checks needed either for BACKEND or for dealing with ARCH variables used in tests/ or lib/, let's discuss first a proper scope for poo#33388 and then figure out where to go from there...
lib/suse_container_urls.pm
40: if (!check_var('ARCH', 'aarch64')) {
50: elsif (is_tumbleweed && (check_var('ARCH', 'i586') || check_var('ARCH', 'x86_64'))) {
54: elsif (is_tumbleweed && check_var('ARCH', 'aarch64')) {
58: elsif (is_tumbleweed && check_var('ARCH', 'ppc64le')) {
62: elsif (is_leap(">15.0") && check_var('ARCH', 'x86_64')) {
66: elsif (is_leap(">15.0") && check_var('ARCH', 'aarch64')) {
70: elsif (is_leap(">15.0") && check_var('ARCH', 'ppc64le')) {
lib/utils.pm
271: elsif (check_var('BACKEND', 's390x')) {
325: return if get_var('BACKEND', '') =~ /ipmi|spvm/;
557: if (check_var('BACKEND', 'svirt') and !check_var('ARCH', 's390x')) {
934: return if check_var('BACKEND', 's390x');
943: systemctl "mask $service_name", ignore_failure => 1 if check_var('BACKEND', 'qemu');
1025: if (check_var('BACKEND', 's390x')) {
1049: if (check_var('BACKEND', 's390x')) {
1083: if (check_var('BACKEND', 'spvm')) {
1089: if (check_var('BACKEND', 'ipmi')) {
1107: if (!check_var('BACKEND', 'svirt')) {