Avoid warning about use of uninitialized value #5540
Conversation
The variable `$testsuite_name` might be `undef` if the spec is an empty hash leading to: ``` Use of uninitialized value $testsuite_name in hash element at /usr/share/openqa/script/../lib/OpenQA/Schema/Result/JobGroups.pm line 413. ``` We can just skip empty specs here avoiding the warning.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5540 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 391 391
Lines 38039 38039
=======================================
Hits 37427 37427
Misses 612 612 ☔ View full report in Codecov by Sentry. |
|
where did you see this error? do you have the according yaml file? |
|
In the log on OSD and no, I don't have the yaml file. |
| @@ -409,7 +409,7 @@ sub _parse_job_template_products ($yaml_products_for_arch, $yaml_defaults_for_ar | |||
| my $description = ''; | |||
| if (ref $spec eq 'HASH') { | |||
| # We only have one key. Asserted by schema | |||
| $testsuite_name = (keys %$spec)[0]; | |||
| next unless $testsuite_name = (keys %$spec)[0]; | |||
There was a problem hiding this comment.
What does this case look like? As the comment even says, this should have been validated beforehand.
There was a problem hiding this comment.
I had a look in the log:
journal:
Mar 21 14:49:58 openqa openqa[29349]: Use of uninitialized value $testsuite_name in hash element at /usr/share/openqa/script/../lib/OpenQA/Schema/Result/JobGroups.pm line 413.
accesslog:
[21/Mar/2024:14:49:58 +0100] "POST /api/v1/job_templates_scheduling/510
[21/Mar/2024:14:49:58 +0100] "POST /api/v1/job_templates_scheduling/319
[21/Mar/2024:14:49:58 +0100] "POST /api/v1/job_templates_scheduling/456
Looking at
https://openqa.suse.de/admin/job_templates/319, it has:
scenarios:
# ...
s390x:
sle-15-SP6-Migration-from-SLE15-SPx-s390x:
- {}
There was a problem hiding this comment.
I suppose an empty hash is considered valid here. It probably might make even sense as a "null value" (and this change would to the right thing then).
There was a problem hiding this comment.
Currently it would still call _parse_job_template_machines() below with an undefined value.
If this is skipped, then the result might be different.
I just had a look at the job template editor and clicked on "Show expanded version and got this:
scenarios:
# ...
s390x:
sle-15-SP6-Migration-from-SLE15-SPx-s390x:
- '':
machine: s390x-kvm
priority: 50
settings: {}
I think with this PR this entry wouldn't be there anymore.
But also an empty string for the testsuite name doesn't make much sense.
I also looked into the database and we have a lot of entries like this with an empty name:
select id, product_id, test_suite_id, t_updated, group_id, name from job_templates where group_id=319 limit 20;
id | product_id | test_suite_id | t_updated | group_id | name
--------+------------+---------------+---------------------+----------+-------------------------------------------------------------------
...
255410 | 2690 | 5576 | 2023-10-19 06:27:17 | 319 |
There was a problem hiding this comment.
FYI I retroactively filed this as poo#157774
|
Superseded by #5583. |
The variable
$testsuite_namemight beundefif the spec is an empty hash leading to:We can just skip empty specs here avoiding the warning.