Project

General

Profile

action #115343

coordination #96596: [qe-core][CI] CI/CD and Coding style improvements

[qe-core] os-autoinst-distri-opensuse checks for record_soft_failure without valid reference is ineffective

Added by okurz about 1 month ago. Updated about 6 hours ago.

Status:
Feedback
Priority:
Normal
Assignee:
Category:
Infrastructure
Target version:
Start date:
2022-08-16
Due date:
% Done:

90%

Estimated time:
Difficulty:

Description

Observation

https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15359/files#diff-899551b3c4ac5af9208cd174eeddc57854657227314a60458fcc9ceccb93c9e4R1258 tries to introduce

        record_soft_failure("Guest $self->{guest_name} unattended installation file hosted on local host can not be reached", "Mark guest installation as FAILED. The unattended installation file url is $self->{guest_installation_automation_file}");

which should have failed in https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/Makefile#L154 due to missing ticket reference.

Expected result

record_soft_failure without issue reference should fail CI checks

Suggestions

  1. Create a commit that adds the line mentioned in the observation (so that the bug is reproduced)
  2. Rewrite the regexp (if needed) to cover the case (https://regex101.com/ can be also used to verify)

Acceptance criteria

  1. static checks are able to capture record_soft_failure without a ticket reference.
  2. existing calls that have no bugref, are replaced with a record_info with a type soft-fail
  3. Create a PR to add the rule to the Contributing.md in the Coding Style section.

History

#1 Updated by szarate 29 days ago

  • Sprint set to QE-Core: September Sprint (Aug 31 - Sep 28)
  • Tags set to qe-core-september-sprint
  • Status changed from New to Workable
  • Priority changed from High to Normal
  • Target version set to QE-Core: Ready

#2 Updated by szarate 29 days ago

#3 Updated by rfan1 19 days ago

  • Status changed from Workable to In Progress
  • Assignee set to rfan1

#4 Updated by rfan1 15 days ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 90

I did some investigation on this ticket.

As per https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15359/files#diff-899551b3c4ac5af9208cd174eeddc57854657227314a60458fcc9ceccb93c9e4R1258

I found record_info is used rather than record_soft_failure, so it may not be covered by CI check.

record_info("Guest $self->{guest_name} unattended installation file hosted on local host can not be reached", "Mark guest installation as FAILED. The unattended installation file url is $self->{guest_installation_automation_file}", result => 'softfail');

BTW, I can see many lines with record_soft_failure have variables, we also handle this:
git --no-pager grep -E -e 'soft_failure>.\;' --and --not -e '([$$0-9a-z]+#[$$0-9a-zA-Z]+|fate.suse.com/[0-9]|\$$[a-z]+)' with *|\$$[a-z]+**

tests/console/cleanup_qam_testrepos.pm:                record_soft_failure("repository $repo is invalid");
tests/console/cleanup_qam_testrepos.pm:                record_soft_failure("removing invalid repository: $repo");
tests/console/journal_check.pm:                record_soft_failure("$bug:\n$buffer");
tests/console/journal_check.pm:                    record_soft_failure("Service: $service failed due to $bsc\n$failed_service_output");
tests/console/ping.pm:            record_soft_failure $bug;
tests/console/python_scientific.pm:            record_soft_failure("$failmsg");
tests/console/snapper_cleanup.pm:            record_soft_failure $msg;
tests/console/yast2_lan_device_settings.pm:            record_soft_failure("Duplicate IP, $static_ip is currently unavailable. Skipping static IP assignment");
tests/console/zfs.pm:            record_soft_failure($msg);
tests/containers/bci_test.pm:        record_soft_failure("The command <tox -e $env> timed out.");
tests/containers/bci_test.pm:        record_soft_failure("The command <tox -e $env> failed.");
tests/kernel/tuned.pm:                record_soft_failure "$bugref - ${known_errors{$_}}";
tests/publiccloud/download_repos.pm:                record_soft_failure("Download failed (rc=$ret):\n$maintrepo");
tests/publiccloud/download_repos.pm:                    record_soft_failure("No .repo file found in $parent. This directory will be removed.");
tests/publiccloud/storage_perf.pm:                record_soft_failure("Deviation occurred. $generic_message");
tests/security/yast2_apparmor/scan_audit_logs.pm:        record_soft_failure "$bsc_number - Wrong value for";
tests/sles4sap/sys_param_check.pm:            record_soft_failure "$bsc_number - Wrong value for $parameter";
tests/virt_autotest/hotplugging_utils.pm:    record_soft_failure("Set live memory failed - expected $memory but got $guestmemory") unless ($within_tolerance);
tests/virt_autotest/sriov_network_card_pci_passthrough.pm:            record_soft_failure "Fail to restore $guest!";
tests/virt_autotest/virt_utils.pm:    record_soft_failure("Not found guest pattern $guest_pattern in $qa_guest_config_file") if ($guest_list eq '');
tests/virt_autotest/virt_utils.pm:            record_soft_failure("$vm_disk_url not found!");
tests/virt_autotest/virt_utils.pm:            record_soft_failure("Failed copying: $mount_point/$remote_guest_xml_file");
tests/virt_autotest/virt_utils.pm:            record_soft_failure("Failed to download: $remote_guest_disk");
tests/virt_autotest/xen_guest_irqbalance.pm:                record_soft_failure("The value of one NIC IRQ smp_affinity, '$_', did not follow the default_smp_affinity, '$default_affinity', with irqbalance enabled.") if $_ ne $default_affinity;

So, current code is good IMO, mark it as Feedback and appreciate if any experts' further comments.

#5 Updated by szarate 9 days ago

  • Description updated (diff)

#6 Updated by rfan1 9 days ago

  • Status changed from Feedback to In Progress
  • % Done changed from 90 to 50

Thanks for the update!
let me try to revise the code.

#7 Updated by rfan1 6 days ago

I can to use the below regexp:

-       @! git --no-pager grep -E -e 'soft_failure\>.*\;' --and --not -e '([$$0-9a-z]+#[$$0-9a-zA-Z]+|fate.suse.com/[0-9]|\$$[a-z]+)' lib/ tests/
+       @! git --no-pager grep -E -e 'soft_failure\>.*\;' --and --not -e '([a-zA-Z]+#[a-zA-Z-]*[0-9]+|fate.suse.com/[0-9]+)' lib/ tests/

However, my question is that, if my PR is merged, how can we handle the exceptions below? IMO the CI check will fail if any new merge requests!

# git --no-pager grep -E -e 'soft_failure\>.*\;' --and --not -e '([a-zA-Z]+#[a-zA-Z-]*[0-9]+|fate.suse.com/[0-9]+)' lib/ tests/
lib/audit_test.pm:        record_soft_failure($msg);
lib/audit_test.pm:                record_soft_failure($msg . "\n" . $reason);
lib/containers/utils.pm:    record_soft_failure("error: $runtime image rmi -a $alpine") if ($output_containers =~ m/Untagged:.*alpine/);
lib/containers/utils.pm:    record_soft_failure("error: $runtime image rmi -a $hello_world:latest") if ($output_containers =~ m/Untagged:.*hello-world:latest/);
lib/known_bugs.pm:                    record_soft_failure $message. "\n\n" . "$line";
lib/known_bugs.pm:                    force_soft_failure $message. "\n\n" . "$line";
lib/opensusebasetest.pm:                record_soft_failure("$bug\n\nDetails:\n$y2log_error_result");
lib/power_action_utils.pm:        record_soft_failure "$args->{soft_failure_reason}";
lib/trento.pm:        record_soft_failure("Cypress exit code:$ret at $log_prefix") if ($ret);
lib/utils.pm:        record_soft_failure "$args{soft_failure_reason}";
lib/utils.pm:            record_soft_failure "Warning: package $pkg_name is not upgraded yet";
lib/virt_autotest/kernel.pm:        record_soft_failure "The $target needs to be checked manually!";
lib/virt_autotest/utils.pm:        record_soft_failure("Failed to download $args{script_name} from $args{script_url}!");
tests/console/cleanup_qam_testrepos.pm:                record_soft_failure("repository $repo is invalid");
tests/console/cleanup_qam_testrepos.pm:                record_soft_failure("removing invalid repository: $repo");
tests/console/journal_check.pm:                record_soft_failure("$bug:\n$buffer");
tests/console/journal_check.pm:                    record_soft_failure("Service: $service failed due to $bsc\n$failed_service_output");
tests/console/ping.pm:            record_soft_failure $bug;
tests/console/python_scientific.pm:            record_soft_failure("$failmsg");
tests/console/snapper_cleanup.pm:            record_soft_failure $msg;
tests/console/yast2_lan_device_settings.pm:            record_soft_failure("Duplicate IP, $static_ip is currently unavailable. Skipping static IP assignment");
tests/console/zfs.pm:            record_soft_failure($msg);
tests/containers/bci_test.pm:        record_soft_failure("The command <tox -e $env> timed out.");
tests/containers/bci_test.pm:        record_soft_failure("The command <tox -e $env> failed.");
tests/kernel/tuned.pm:                record_soft_failure "$bugref - ${known_errors{$_}}";
tests/publiccloud/download_repos.pm:                record_soft_failure("Download failed (rc=$ret):\n$maintrepo");
tests/publiccloud/download_repos.pm:                    record_soft_failure("No .repo file found in $parent. This directory will be removed.");
tests/publiccloud/storage_perf.pm:                record_soft_failure("Deviation occurred. $generic_message");
tests/sles4sap/sapconf.pm:            record_soft_failure "$1#$2";
tests/sles4sap/sys_param_check.pm:            record_soft_failure "$bsc_number - Wrong value for $parameter";
tests/virt_autotest/hotplugging_utils.pm:    record_soft_failure("Set live memory failed - expected $memory but got $guestmemory") unless ($within_tolerance);
tests/virt_autotest/sriov_network_card_pci_passthrough.pm:            record_soft_failure "Fail to restore $guest!";
tests/virt_autotest/virt_utils.pm:    record_soft_failure("Not found guest pattern $guest_pattern in $qa_guest_config_file") if ($guest_list eq '');
tests/virt_autotest/virt_utils.pm:            record_soft_failure("$vm_disk_url not found!");
tests/virt_autotest/virt_utils.pm:            record_soft_failure("Failed copying: $mount_point/$remote_guest_xml_file");
tests/virt_autotest/virt_utils.pm:            record_soft_failure("Failed to download: $remote_guest_disk");
tests/virt_autotest/xen_guest_irqbalance.pm:                record_soft_failure("The value of one NIC IRQ smp_affinity, '$_', did not follow the default_smp_affinity, '$default_affinity', with irqbalance enabled.") if $_ ne $default_affinity;
tests/virt_autotest/xen_guest_irqbalance.pm:                record_soft_failure("IRQ are not balanced as the vif interrupts for CPU" . $cpu_id . " is " . $increased_irqs_on_cpu[$cpu_id]);
tests/virt_autotest/xen_guest_irqbalance.pm:            record_soft_failure "Fail to restore $guest!";
tests/virtualization/universal/guest_management.pm:            record_soft_failure "Reboot on $guest failed";
tests/virtualization/universal/guest_management.pm:            record_soft_failure "Guest $guest is not running after the reboot";
tests/virtualization/universal/guest_management.pm:            record_soft_failure "Guest $guest seems to be already down";
tests/virtualization/universal/guest_management.pm:            record_soft_failure "Shutdown on $guest failed";
tests/virtualization/universal/hotplugging_HDD.pm:                record_soft_failure("lsblk failed - please check the output manually") if $lsblk != 0;
tests/virtualization/universal/hotplugging_HDD.pm:                record_soft_failure("lsblk failed - please check the output manually") if $lsblk != 0;
tests/virtualization/universal/hotplugging_HDD.pm:                record_soft_failure($msg);
tests/virtualization/universal/list_guests.pm:            record_soft_failure("enure_online failed for $guest: $err");
tests/virtualization/universal/save_and_restore.pm:            record_soft_failure "Guest $guest should be shut down now";
tests/virtualization/universal/smoketest.pm:                record_soft_failure("$cve ($name) vulnerable on $go_to_target") unless ignore_cve_fail($cve, $go_to_target);
tests/virtualization/universal/stresstest.pm:                record_soft_failure "stresstest failed on $guest";
tests/virtualization/universal/virsh_start.pm:            record_soft_failure "Cannot enable autostart on $guest guest";
tests/virtualization/universal/waitfor_guests.pm:        record_soft_failure($msg);

Any suggestions?

#8 Updated by szarate about 8 hours ago

  • Sprint changed from QE-Core: September Sprint (Aug 31 - Sep 28) to QE-Core: October Sprint (Sep 28 - Oct 26)

#9 Updated by szarate about 7 hours ago

#10 Updated by szarate about 7 hours ago

  • Parent task set to #96596

#12 Updated by rfan1 about 6 hours ago

  • % Done changed from 50 to 80

#13 Updated by rfan1 about 6 hours ago

  • Status changed from In Progress to Feedback
  • % Done changed from 80 to 90

Also available in: Atom PDF