Bug 40404 - t/Test/Mock/Logger.t generates warnings
Summary: t/Test/Mock/Logger.t generates warnings
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 38818
Blocks: 40444
  Show dependency treegraph
 
Reported: 2025-07-16 09:07 UTC by Jonathan Druart
Modified: 2025-08-01 13:23 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.02,24.11.08
Circulation function:


Attachments
Bug 40404: Remove diag from t/Test/Mock/Logger.t (2.40 KB, patch)
2025-07-16 09:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40404: Remove diag from t/Test/Mock/Logger.t (2.45 KB, patch)
2025-07-18 07:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 40404: Remove diag from t/Test/Mock/Logger.t (2.51 KB, patch)
2025-07-18 12:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-07-16 09:07:43 UTC
t/Test/Mock/Logger.t .. 1/10     # trace:                                                                 
    #    (No trace messages)                                                                                                                                                                                         
    # debug:                                                                                                                                                                                                         
    #     "Debug message" at /usr/share/perl/5.36/Test/Builder.pm line 374                                
    # info:                                                                                                                                                                                                          
    #    (No info messages)                                                                               
    # warn:                                                                                                                                                                                                          
    #    (No warn messages)                                                                                                                                                                                          
    # error:                                                                                              
    #    (No error messages)                                                                              
    # fatal:                                                                                              
    #    (No fatal messages)                                                                                                                                                                                         
t/Test/Mock/Logger.t .. ok                                                                                                                                                                                           
All tests successful.                                                                                     
Files=1, Tests=10,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.40 cusr  0.07 csys =  0.50 CPU)            
Result: PASS
Comment 1 Jonathan Druart 2025-07-16 09:09:46 UTC
Created attachment 184127 [details] [review]
Bug 40404: Remove diag from t/Test/Mock/Logger.t

t/Test/Mock/Logger.t .. 1/10     # trace:
    #    (No trace messages)
    # debug:
    #     "Debug message" at /usr/share/perl/5.36/Test/Builder.pm line 374
    # info:
    #    (No info messages)
    # warn:
    #    (No warn messages)
    # error:
    #    (No error messages)
    # fatal:
    #    (No fatal messages)
t/Test/Mock/Logger.t .. ok
All tests successful.
Files=1, Tests=10,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.40 cusr  0.07 csys =  0.50 CPU)
Result: PASS

This one is a bit tricky: we cannot use Test::Warn to capture the output because Test::Builder->diag does not write directly to STDERR.
It has its own failure_output handle (which defaults to STDERR) but bypasses Perl’s standard layers.
Comment 2 David Nind 2025-07-18 07:55:36 UTC
Created attachment 184296 [details] [review]
Bug 40404: Remove diag from t/Test/Mock/Logger.t

t/Test/Mock/Logger.t .. 1/10     # trace:
    #    (No trace messages)
    # debug:
    #     "Debug message" at /usr/share/perl/5.36/Test/Builder.pm line 374
    # info:
    #    (No info messages)
    # warn:
    #    (No warn messages)
    # error:
    #    (No error messages)
    # fatal:
    #    (No fatal messages)
t/Test/Mock/Logger.t .. ok
All tests successful.
Files=1, Tests=10,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.40 cusr  0.07 csys =  0.50 CPU)
Result: PASS

This one is a bit tricky: we cannot use Test::Warn to capture the output because Test::Builder->diag does not write directly to STDERR.
It has its own failure_output handle (which defaults to STDERR) but bypasses Perl’s standard layers.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2025-07-18 09:56:29 UTC
This looks a bit weird and complex for what it should do. And it is/was called test diag output. But it seems to do something else? Like checking the former debug call?

Please clarify what we want to achieve here and if this is the simplest way to do it.
Comment 4 Jonathan Druart 2025-07-18 10:17:51 UTC
(In reply to Marcel de Rooy from comment #3)
> This looks a bit weird and complex for what it should do. And it is/was
> called test diag output. But it seems to do something else? Like checking
> the former debug call?
> 
> Please clarify what we want to achieve here and if this is the simplest way
> to do it.

The diag is not directly used, the different occurrences in the code can be found with `git grep '$logger->diag' t`.

I don't really understand why we need it, but it seems useful in some context (re bug 38818).

As I tried to explain in the commit message, we cannot use the regular warning_is from Test::Warn because diag will not write directly to STDERR.

It's the only way I've found to remove the warning and be able to use NoWarnings in this test file.
Comment 5 Martin Renvoize (ashimema) 2025-07-18 12:11:56 UTC
Created attachment 184361 [details] [review]
Bug 40404: Remove diag from t/Test/Mock/Logger.t

t/Test/Mock/Logger.t .. 1/10     # trace:
    #    (No trace messages)
    # debug:
    #     "Debug message" at /usr/share/perl/5.36/Test/Builder.pm line 374
    # info:
    #    (No info messages)
    # warn:
    #    (No warn messages)
    # error:
    #    (No error messages)
    # fatal:
    #    (No fatal messages)
t/Test/Mock/Logger.t .. ok
All tests successful.
Files=1, Tests=10,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.40 cusr  0.07 csys =  0.50 CPU)
Result: PASS

This one is a bit tricky: we cannot use Test::Warn to capture the output because Test::Builder->diag does not write directly to STDERR.
It has its own failure_output handle (which defaults to STDERR) but bypasses Perl’s standard layers.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 6 Lucas Gass (lukeg) 2025-07-18 17:12:43 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 7 Paul Derscheid 2025-07-24 08:21:32 UTC
Nice work everyone!

Pushed to 25.05.x
Comment 8 Fridolin Somers 2025-08-01 13:23:21 UTC
Pushed to 24.11.x for 24.11.08