[wp-trac] [WordPress Trac] #18088: Network Admin Email setting in wp-admin/network/settings.php fails silently

WordPress Trac noreply at wordpress.org
Fri Sep 30 05:32:29 UTC 2016


#18088: Network Admin Email setting in wp-admin/network/settings.php fails silently
-------------------------------------+-------------------------------------
 Reporter:  kawauso                  |       Owner:  jeremyfelt
     Type:  defect (bug)             |      Status:  reviewing
 Priority:  normal                   |   Milestone:  4.7
Component:  Networks and Sites       |     Version:  3.2
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch has-unit-      |     Focuses:  administration,
  tests                              |  multisite
-------------------------------------+-------------------------------------

Comment (by jeremyfelt):

 Replying to [comment:28 flixos90]:
 > [attachment:18088.5.diff] adds unit tests for `get_settings_errors()`
 (which didn't exist before). The new functionality is also included in one
 of the tests.

 Nice and creative, @flixos90. This is looking doable. :)

 A couple notes:

 Globals in tests are terrifying. If not managed properly they will bleed
 into other tests and cause strange bugs. In tests like these, make sure
 that the globals modified in the test are set back to their original value
 before the assertion is run. If the assertion was to fail, the remainder
 of the function would not run and the global would be stuck as is. This
 could cause false positives or negatives in the tests that run after the
 failed assertion.

 {{{
 function test_result() {
     $_old_global = $global_data;
     $result = get_result();
     $global_data = $_old_global;

     $this->assertEquals( 'expected', $result );
 }
 }}}

 With that same concept in mind, it's also better to split these tests up
 so that only one (or as few as possible) assertions run in each. If the
 first assertion was to fail in `test_get_settings_errors()`, the next 2
 would not run. This could cause other issues or could just mask a full
 picture of the result.

 `test_get_settings_errors()` here could be split into 3 methods:
 * `test_get_settings_errors_retrieve_single_error_code()`
 * `test_get_settings_errors_retrieve_all_error_codes()`
 * `test_get_settings_errors_retrieve_invalid_error_code()`

 From time to time it probably doesn't make sense to split up the tests,
 but it's a good approach to start with. (IMO)

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


More information about the wp-trac mailing list