Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
75bf9d1
#563 - set wp image editor quality for mime types according to wp ver…
mehulkaklotar Nov 1, 2022
a2c34d0
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 1, 2022
52b8ede
#563 - fix older tests to accommodate new changes
mehulkaklotar Nov 1, 2022
59a3b4c
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 1, 2022
0ddc6d9
Merge remote-tracking branch 'origin/enhancement/563-image-editor-qua…
mehulkaklotar Nov 1, 2022
b4e342d
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 7, 2022
3625323
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 8, 2022
c3b625f
return early for non image mimes and other fixes
mehulkaklotar Nov 8, 2022
264ae36
unit test added to check filter adds the quality or not
mehulkaklotar Nov 8, 2022
80c5381
fixes for wp version added
mehulkaklotar Nov 8, 2022
e05b4ec
fixes for wp version added tests
mehulkaklotar Nov 8, 2022
0df8f1a
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 8, 2022
f4f022f
fixes for wp version added tests
mehulkaklotar Nov 8, 2022
8cd2490
fix for 6.1 and lower for webp and universal quality added
mehulkaklotar Nov 9, 2022
eeace67
fix test for 6.1 and lower for webp and universal quality added
mehulkaklotar Nov 9, 2022
929ba07
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 9, 2022
4af5bc9
Update tests/modules/images/webp-uploads/load-tests.php
mehulkaklotar Nov 9, 2022
039d484
fix tests for the images with quality
mehulkaklotar Nov 9, 2022
07917e3
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 10, 2022
0d75cfb
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 10, 2022
8d3bcc7
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 10, 2022
7cfcd72
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 10, 2022
58b5e9b
Update modules/images/webp-uploads/load.php
mehulkaklotar Nov 10, 2022
af18717
fix tests
mehulkaklotar Nov 11, 2022
7dd81d7
fix tests
mehulkaklotar Nov 15, 2022
aab1208
Merge branch 'trunk' into enhancement/563-image-editor-quality
mehulkaklotar Nov 15, 2022
0fb1e19
fix tests
mehulkaklotar Nov 16, 2022
b1f3778
Merge remote-tracking branch 'origin/enhancement/563-image-editor-qua…
mehulkaklotar Nov 16, 2022
d3f97eb
Revert "fix tests"
mehulkaklotar Nov 16, 2022
f268b54
fix tests with tear down method
mehulkaklotar Nov 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix tests for the images with quality
  • Loading branch information
mehulkaklotar committed Nov 9, 2022
commit 039d4849b7562b47e82616835ca2dcb8fca81ff6
35 changes: 11 additions & 24 deletions tests/modules/images/webp-uploads/load-tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -926,32 +926,33 @@ public function it_should_create_mime_types_for_allowed_sizes_only_via_global_va
* @test
*/
public function it_should_set_quality_with_image_conversion() {
global $wp_version;
// Set conversions for uploaded images.
add_filter( 'image_editor_output_format', array( $this, 'image_editor_output_formats' ) );

$editor = wp_get_image_editor( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/dice.png', array( 'mime_type' => 'image/png' ) );

// Quality setting for the source image. For PNG the fallback default of 82 is used.
$this->assertSame( 82, $editor->get_quality(), 'Default quality setting for PNG is 82.' );

$file = wp_tempnam();

// A PNG image will be converted to WebP whose quality should be 82 universally.
$editor->save();
$this->assertSame( 82, $editor->get_quality(), 'Output image format is WEBP. Quality setting for it should be 82 universally.' );
$editor->save( $file, 'image/webp' );
$this->assertSame( 82, $editor->get_quality(), 'Output image format is WebP. Quality setting for it should be 82 universally.' );

unlink( $file );
unset( $editor );

$editor = wp_get_image_editor( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' );

// Quality setting for the source image. For JPG the fallback default of 82 is used.
$this->assertSame( 82, $editor->get_quality(), 'Default quality setting for JPG is 82.' );

// A JPG image will be converted to WEBP whose quality should be 82 universally.
$editor->save();
$this->assertSame( 82, $editor->get_quality(), 'Output image format is WEBP. Quality setting for it should be 82 universally.' );
$file = wp_tempnam();

// A JPG image will be converted to WebP whose quality should be 82 universally.
$editor->save( $file, 'image/webp' );
$this->assertSame( 82, $editor->get_quality(), 'Output image format is WebP. Quality setting for it should be 82 universally.' );

unlink( $file );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code will not run if any of the above assertion fails. See my comment above though, I think we should not use a temporary file anyway, so I think the code here should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment above about test files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mehulkaklotar I understand now why you're using wp_tempnam(), that makes sense. However, the comment here still needs to be addressed: If an assertion fails, this line is never reached, which means it would leave the file in place.

We need to change this as follows:

  • Assign the results from any method calls from assertions here to variables (before the unlink call).
  • Run all assertions only after the unlink call.

This way, when an assertion fails, at least the file was still deleted as usual.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixarntz Another alternative could be to store the file name in a property and clean it up in a tear down.

Example class
class TestCase { // could also be implemented as a trait

	protected $to_unlink = [];
	
	public function test_something_with_a_file() {
		$filename = $this->temp_filename();
		
		file_put_contents( $filename, 'foobar' );
		
		// $filename will always be deleted after the test
	}

	protected function temp_filename() {
		$filename = wp_tempnam();

		$this->to_unlink[] = $filename;

		return $filename;
	}

	public function tear_down() {
		$this->to_unlink = array_filter(
			$this->to_unlink,
			function ( $filename ) {
				return unlink( $filename );
			}
		);
	}

}

unset( $editor );
remove_filter( 'image_editor_output_format', array( $this, 'image_editor_output_formats' ) );
}

/**
Expand All @@ -961,18 +962,4 @@ private function mock_frontend_body_hooks() {
remove_all_actions( 'template_redirect' );
do_action( 'template_redirect' );
}

/**
* Changes the output format when editing images. PNG and JPEG files
* will be converted to WEBP (if the image editor in PHP supports it).
*
* @param array $formats Output formats.
*
* @return array
*/
public function image_editor_output_formats( $formats ) {
$formats['image/png'] = 'image/webp';
$formats['image/jpeg'] = 'image/webp';
return $formats;
}
}