Project

General

Profile

action #10704

Make warnings in tests fatal

Added by okurz over 5 years ago. Updated 8 months ago.

Status:
New
Priority:
Low
Assignee:
-
Category:
Feature requests
Target version:
Start date:
2016-02-10
Due date:
% Done:

0%

Estimated time:
Difficulty:

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


Related issues

Related to openQA Project - action #78240: prevent circular dependencies in bmwqemu.pm and autotest.pm to be able to use "strictures"New2020-11-19

History

#1 Updated by okurz over 5 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?

#2 Updated by okurz over 5 years ago

  • Description updated (diff)

#3 Updated by okurz over 5 years ago

  • Status changed from New to In Progress
  • Assignee set to okurz

PR ready gh#570

#4 Updated by okurz over 5 years ago

  • Status changed from In Progress to Resolved

merged, done

#5 Updated by okurz over 5 years ago

  • Status changed from Resolved to In Progress

no, wait, need to do the same for os-autoinst

#6 Updated by okurz over 5 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 :-)

#7 Updated by okurz over 4 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.

#8 Updated by rpalethorpe over 4 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.

#9 Updated by coolo over 3 years ago

  • Status changed from In Progress to New
  • Target version set to future

#10 Updated by okurz about 3 years ago

  • Target version changed from future to future

#11 Updated by okurz about 2 years ago

  • Category changed from 132 to Feature requests

#12 Updated by okurz 12 months 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.

#13 Updated by okurz 12 months 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.

#14 Updated by cdywan 11 months 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).

#15 Updated by okurz 11 months 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 :)

#16 Updated by okurz 10 months 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

#17 Updated by cdywan 9 months ago

Are you still pursuing the proof of concept with strictures?

#18 Updated by okurz 9 months 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 :)

#19 Updated by okurz 8 months ago

  • Related to action #78240: prevent circular dependencies in bmwqemu.pm and autotest.pm to be able to use "strictures" added

#20 Updated by okurz 8 months ago

  • Status changed from Feedback to New
  • Assignee deleted (okurz)

blocked by new ticket #78240

Also available in: Atom PDF