action #10704
openMake warnings in tests fatal
0%
Description
user story¶
As an openQA developer I want openQA unit and integration tests to fail when warnings are encountered to ensure not introduce regressions
acceptance criteria¶
- "make test" fails if new warnings are encountered
- the existing warnings are fixed or properly marked and skipped
tasks¶
- It should help if we first fix all existing warnings to make new ones more visible
- research how to make "prove" catch warnings and make them fatal
- why does "prove -W" ("") not work as I expect it?
- decide on an approach
- make warnings fatal in all tests
- fix existing warnings
further notes¶
- http://www.modernperlbooks.com/mt/2014/01/fatal-warnings-are-a-ticking-time-bomb.html
- http://stackoverflow.com/questions/1353179/how-should-i-promote-perl-warnings-to-fatal-errors-during-development
- http://www.perlmonks.org/?node_id=324749
- http://perldoc.perl.org/prove.html
- http://perldoc.perl.org/warnings.html#Fatal-Warnings
- http://www.perlmonks.org/?node_id=931025
- http://perlmaven.com/always-use-strict-and-use-warnings
- http://search.cpan.org/~skington/warnings-everywhere-0.006/lib/warnings/everywhere.pm
Updated by okurz over 8 years ago
see here for a potential starting point for discussion: https://github.com/os-autoinst/openQA/pull/539
How should we go on from there?
Updated by okurz over 8 years ago
- Status changed from New to In Progress
- Assignee set to okurz
PR ready gh#570
Updated by okurz over 8 years ago
- Status changed from Resolved to In Progress
no, wait, need to do the same for os-autoinst
Updated by okurz over 8 years ago
- Assignee deleted (
okurz)
on hold, anyone else can take over and fix that for os-autoinst. easy task as openQA is already done and can be used as reference :-)
Updated by okurz over 7 years ago
- Assignee set to rpalethorpe
as mentioned in https://github.com/os-autoinst/os-autoinst/pull/716#discussion_r100562659 rpalethorphe stepped up to look into some warnings.
Updated by rpalethorpe over 7 years ago
- Assignee deleted (
rpalethorpe)
I just removed my custom code to catch warnings and added Test::Warnings to the virtio terminal tests. https://github.com/os-autoinst/os-autoinst/pull/722 Seems to work fine!
And now I am stepping down.
Updated by coolo over 6 years ago
- Status changed from In Progress to New
- Target version set to future
Updated by okurz about 6 years ago
- Target version changed from future to future
Updated by okurz about 5 years ago
- Category changed from 132 to Feature requests
Updated by okurz almost 4 years ago
- Priority changed from Normal to Low
We use Test::Warnings in most test modules within os-autoinst as well as openQA with good success. We do not have a check that Test::Warnings is used everywhere and also we do not have good reporting about warnings from tests or so, this can still be done.
Updated by okurz almost 4 years ago
- Status changed from New to Feedback
- Assignee set to okurz
Found https://stackoverflow.com/a/1353235
Created https://github.com/os-autoinst/os-autoinst/pull/1497 to make all perl warnings encountered from test distributions fatal with a switch to be able to disable that. created https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/10764 to fix a perl warning I found in very common jobs, e.g. https://openqa.opensuse.org/tests/1348002/file/autoinst-log.txt from the "gnome" test suite.
SLE tests look worse, e.g. https://openqa.suse.de/tests/4512233/file/autoinst-log.txt for "sle-15-SP2-Server-DVD-HPC-Incidents-x86_64-hpc_conman@64bit" showing problems in lib/main_common about qam incidents. Also covered in the above PR now.
Updated by livdywan almost 4 years ago
I suppose the conversation is still on-going with regard to the implications of making runtime warnings fatal and how to expose them (as can be seen in the PR review comments).
Updated by okurz almost 4 years ago
you can say so. The last comment was from just 5 days ago and I wanted to read about what Linus wrote in the linked mailing list post :)
Updated by okurz almost 4 years ago
kraih pointed out https://metacpan.org/pod/strictures which we have available in openSUSE as perl-strictures already. This looks even better.
Trying with https://github.com/os-autoinst/os-autoinst/pull/1548
Updated by livdywan over 3 years ago
Are you still pursuing the proof of concept with strictures?
Updated by okurz over 3 years ago
well, no action was done since the last comment and nothing changed. I could just unassign but I still have my ticket open and it's in "future" so I think no one else is more likely to pick it up when it's done and the ticket is "Low" so I don't think anything needs to be changed. Unassigning and eventually reassigning would just be noise. As long as you still consider myself a useful member of the team I guess we can keep it this way :)
Updated by okurz over 3 years ago
- Related to action #78240: prevent circular dependencies in bmwqemu.pm and autotest.pm to be able to use "strictures" added
Updated by okurz over 3 years ago
- Status changed from Feedback to New
- Assignee deleted (
okurz)
blocked by new ticket #78240
Updated by tinita over 2 years ago
- Copied to action #108503: Collect and prominently display warnings from autoinst.log added