Use require for loading OpenQA::Test::Database #5756
Conversation
It does a
plan skip_all ...
in its main code, which is effectively an exit.
Since that happens in a BEGIN block currently during a 'use', it won't execute
any INIT blocks any more in perl version >= 5.38. But the -c compile
check is running in an INIT block.
Issue: https://progress.opensuse.org/issues/162866
There was a problem hiding this comment.
Please rephrase the git commit message subject line to state the "Why", not the "What".
Now seeing the code I remember that already in many other cases we applied the best practice to not use any Test::More calls in test helper libraries so maybe we can do that instead? So replace the "plan skip_all" in OpenQA::Test::Database with a "warn" and empty return maybe? As we check for warnings in tests that would still prevent that tests are not executed unnoticed.
| @@ -10,7 +10,7 @@ use Test::MockModule; | |||
| use Test::Mojo; | |||
| use Test::Output 'combined_like'; | |||
| use Test::Warnings; | |||
| use OpenQA::Test::Database; | |||
| require OpenQA::Test::Database; | |||
There was a problem hiding this comment.
I suggest to add a comment in each line to explain why we use require as exception, e.g.
| require OpenQA::Test::Database; | |
| require OpenQA::Test::Database; # "require" is needed due to "plan skip_all" in BEGIN |
There was a problem hiding this comment.
Also that's a bit inaccurate. It's to avoid happening plan skip_all in BEGIN
|
Feel free to take over the ticket |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5756 +/- ##
==========================================
- Coverage 98.43% 98.43% -0.01%
==========================================
Files 393 393
Lines 38594 38593 -1
==========================================
- Hits 37991 37990 -1
Misses 603 603 ☔ View full report in Codecov by Sentry. |
|
But yes, the commit message should be something like "Fix compile checks ..." to state the why. I don't think adding a comment on every require is necessary, though. Maybe we can enforce this via a style check or make our even add tests for this. |
It does a
in its main code, which is effectively an exit.
Since that happens in a BEGIN block currently during a 'use', it won't execute any INIT blocks any more in perl version >= 5.38. But the -c compile check is running in an INIT block.
Issue: https://progress.opensuse.org/issues/162866