[wp-trac] [WordPress Trac] #65181: Tests: Add unit tests for show_message()

[WordPress Trac]

#65181: Tests: Add unit tests for show_message()
----------------------------+------------------------------
 Reporter:  pbearne         |       Owner:  (none)
     Type:  enhancement     |      Status:  new
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  Administration  |     Version:
 Severity:  normal          |  Resolution:
 Keywords:  dev-feedback    |     Focuses:
----------------------------+------------------------------
Changes (by pbearne):

 * keywords:   => dev-feedback


Comment:

 The show_message() function is currently difficult to test because it has
 two major side effects:
 1. Direct Output: It uses echo to print the message, requiring output
 buffering to capture it.
 2. Buffer Flushing: It calls wp_ob_end_flush_all() and flush(), which
 force-closes all active output buffers. This interferes with PHPUnit's own
 output handling and makes it impossible to reliably use ob_start() within
 a test to capture the output.
 **Proposed Refactoring**
 To make show_message() testable while maintaining backward compatibility,
 we can split the formatting logic from the output logic.
 Option 1: Add a return parameter (Recommended) We can add an optional
 second parameter $echo that defaults to true. When set to false, the
 function returns the HTML string instead of printing and flushing.
 {{{#!php
 <?php
 function show_message( $message, $echo = true ) {
     if ( is_wp_error( $message ) ) {
         if ( $message->get_error_data() && is_string(
 $message->get_error_data() ) ) {
             $message = $message->get_error_message() . ': ' .
 $message->get_error_data();
         } else {
             $message = $message->get_error_message();
         }
     }

     $output = "<p>$message</p>\n";

     if ( ! $echo ) {
         return $output;
     }

     echo $output;
     wp_ob_end_flush_all();
     flush();
 }
 }}}

 Option 2: Extract a "get" version Create a new function get_show_message()
 that handles the string conversion and wrapping, and have show_message()
 call it.
 {{{#!php
 <?php
 function get_show_message( $message ) {
     if ( is_wp_error( $message ) ) {
         if ( $message->get_error_data() && is_string(
 $message->get_error_data() ) ) {
             return $message->get_error_message() . ': ' .
 $message->get_error_data();
         }
         return $message->get_error_message();
     }
     return $message;
 }

 function show_message( $message ) {
     echo "<p>" . get_show_message( $message ) . "</p>\n";
     wp_ob_end_flush_all();
     flush();
 }
 }}}

 Why Option 1 is better for WordPress Core:

 * It's a common pattern in WordPress (e.g., wp_dropdown_categories(),
 get_the_tag_list()).
 * It allows tests to call show_message( $msg, false ) and assert the
 returned string without causing "risky" test warnings in PHPUnit.
 * It doesn't introduce a new global function that might have naming
 conflicts or limited utility.
 Would you like me to implement one of these changes and update the tests
 accordingly?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/65181#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform