Cleanup like this should happen in a tearDown method as it will not run if an assertion fails before it as part of the test body. These references can be stored in a property that is processed later in the tear down method. You might also consider splitting into separate test methods if such cleanup is needed before each "inner test".

Why is this unsetting of variables even needed? They are just rewritten right below. It seems to me that these two lines could simply be removed.

The unset for $file shouldn't be needed, but my guess regarding the $editor was to invoke \WP_Image_Editor_Imagick::__destruct or another classes' destructor. I'm not sure if this would be called immediately otherwise just by overwriting the variable. Again, I don't think it would be necessary with separate tests but if it were for some reason, it should get a comment to explain that, otherwise it doesn't make much sense 👍

unset was not required there. Unlink is still required for temporary file that is being generated with image editor save, but that is also needed at the end of test case. I have removed the in between unset and unlink code now.

Why is this unsetting of variables even needed? They are just rewritten right below. It seems to me that these two lines could simply be removed.

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.

see my comment above about test files.

@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.

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

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 );
			}
		);
	}

}

@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.