Bug 40404

Summary: t/Test/Mock/Logger.t generates warnings
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: Pushed to oldstable --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, m.de.rooy, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
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:
Bug Depends on: 38818    
Bug Blocks: 40444    
Attachments: Bug 40404: Remove diag from t/Test/Mock/Logger.t
Bug 40404: Remove diag from t/Test/Mock/Logger.t
Bug 40404: Remove diag from t/Test/Mock/Logger.t

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