action #115343
closedcoordination #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
100%
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¶
- Create a commit that adds the line mentioned in the observation (so that the bug is reproduced)
- Rewrite the regexp (if needed) to cover the case (https://regex101.com/ can be also used to verify)
Acceptance criteria¶
- static checks are able to capture
record_soft_failure
without a ticket reference. - existing calls that have no bugref, are replaced with a
record_info
with a type soft-fail - Create a PR to add the rule to the Contributing.md in the Coding Style section.
Updated by szarate about 2 years 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
Updated by szarate about 2 years ago
- Related to coordination #96596: [qe-core][CI] CI/CD and Coding style improvements added
Updated by rfan1 about 2 years ago
- Status changed from Workable to In Progress
- Assignee set to rfan1
Updated by rfan1 about 2 years ago
- Status changed from In Progress to Feedback
- % Done changed from 0 to 90
I did some investigation on this ticket.
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.
Updated by rfan1 about 2 years 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.
Updated by rfan1 about 2 years 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?
Updated by szarate about 2 years ago
- Sprint changed from QE-Core: September Sprint (Aug 31 - Sep 28) to QE-Core: October Sprint (Sep 28 - Oct 26)
Updated by szarate about 2 years ago
- Related to deleted (coordination #96596: [qe-core][CI] CI/CD and Coding style improvements)
Updated by rfan1 about 2 years ago
Updated by rfan1 about 2 years ago
- Status changed from In Progress to Feedback
- % Done changed from 80 to 90
Updated by rfan1 about 2 years ago
- Status changed from Feedback to In Progress
The previous PR was reverted due to https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15612
Now fix it with new one:
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15615/