[wp-trac] [WordPress Trac] #26823: wp_get_image_editor->multi_resize()

[WordPress Trac]

#26823: wp_get_image_editor->multi_resize()
--------------------------+------------------
 Reporter:  pbearne       |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  3.9
Component:  Media         |     Version:  3.5
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:
--------------------------+------------------

Comment (by DH-Shredder):

 Replying to [comment:40 pbearne]:
 > Replying to [comment:39 DH-Shredder]:
 > > This is still missing a test with both width and height set to null
 (and no image being generated as a result).
 > >
 > > As I may not get to this tonight, feel free to add one if you get to
 it first.  :)
 >
 >
 > Will do

 Awesome, thanks!

 > I feel that we make sure that we don't have file with the expected value
 in the target folder so I feel we should add this before the function is
 run

 It seems pretty safe to assume that the files won't be there when we start
 it, since if there are files left, it means there was a failure somewhere.
 If it passes once, the files get cleared up.  In other unit/integration
 tests in core that I checked, we currently only remove filesystem content
 after the test is complete, rather than pre-cleaning.

 However! I'm willing to defer on that point to someone who has done more
 testing than I.

 >

 > I see a couple of other values that could be used
 >
 >
 > {{{
 > //will create and image with the size in the filename
 >                       array(
 >                               'width'  => 9999, # Arbitrary High Value
 >                               'height' => 9999, # Arbitrary High Value
 >                       ),
 > // will this crop the image to the max size
 >                       array(
 >                               'width'  => 9999, # Arbitrary High Value
 >                               'height' => 9999, # Arbitrary High Value
 >                               'crop'   => true,
 >                       ),
 > //will create and image with the size in the filename
 >                       array(
 >                               'width'  => 0, # Arbitrary High Value
 >                               'height' => 0, # Arbitrary High Value
 >                       ),
 > // will this crop the image
 >                       array(
 >                               'width'  => 0, # Arbitrary High Value
 >                               'height' => 0, # Arbitrary High Value
 >                               'crop'   => true,
 >                       ),
 > }}}

 Agreed that that sort of additional tests are a good idea. However, we
 have to be cautious with having those in the same (long) test. If there's
 more than one image in a `multi_resize` call that intentionally does not
 create a new file or Array entry, it will make it harder to tell which one
 failed, and for what reason, when we run the batch.

 As a last note, I realize it was probably just a quick copy/paste (which
 I've certainly done before), but I'd suggest being careful that the inline
 comments match what you're trying to test (`0` is not an `arbitrarily high
 value`) -- it helps to make it as clear as possible which behaviour that
 is being tested, so that if/when the test fails, it's obvious what went
 wrong. :)

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