Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#42736 closed defect (bug) (fixed)

Avoid hard-coded example.org in test_media_handle_upload_uses_post_parent_for_directory_date() and fix a "false positive" assertion in test_media_handle_upload_ignores_page_parent_for_directory_date()

Reported by: birgire's profile birgire Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.9
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

When I ran

phpunit --group media

on the latest wordpress-develop install with:

define( 'WP_TESTS_DOMAIN', 'test.dev' );

I noticed two things related to the changeset in [41964]:

The first one is that the assertion in
test_media_handle_upload_uses_post_parent_for_directory_date() assumes the example.org test domain:

There was 1 failure:

1) Tests_Media::test_media_handle_upload_uses_post_parent_for_directory_date
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'http://example.org/wp-content/uploads/2010/01/test-image-iptc.jpg'
+'http://test.dev/wp-content/uploads/2010/01/test-image-iptc.jpg'

/full/path/to/tests/phpunit/tests/media.php:2285


The second thing is in Tests_Media::test_media_handle_upload_ignores_page_parent_for_directory_date() where we have:

$uploads_dir = wp_upload_dir( current_time( 'mysql' ) );

$expected = $uploads_dir['url'] . 'test-image-iptc.jpg';

This generates $expected as:

http://test.dev/wp-content/uploads/2017/11test-image-iptc.jpg

but the actual $url generated within the method is:

http://test.dev/wp-content/uploads/2017/11/test-image-iptc.jpg

So it satisfies:

$this->assertNotEquals( $expected, $url ); 


but it looks like a / is missing from $expected and we should add it and then assert if:

$this->assertSame( $expected, $url );


making sure it's placed in the current month's upload directory, not the 2010/01 upload directory, because without the patched page post type exclusion, the actual $url would become:

http://test.dev/wp-content/uploads/2010/01/test-image-iptc.jpg

I hope I'm understanding it correctly ;-)

Attachments (1)

42736.diff (998 bytes) - added by birgire 7 years ago.

Download all attachments as: .zip

Change History (7)

@birgire
7 years ago

#1 @birgire
7 years ago

  • Keywords has-patch added

42736.diff contains suggested updates to:

  • Tests_Media::test_media_handle_upload_uses_post_parent_for_directory_date(): adds a missing slash and supports WP_TESTS_DOMAIN rather than the fixed example.org test domain.
  • Tests_Media::test_media_handle_upload_ignores_page_parent_for_directory_date(): adds a missing slash and asserts that the file is from the current month's upload directory, not a different one.
Version 0, edited 7 years ago by birgire (next)

#2 @birgire
7 years ago

  • Summary changed from Adjust two media test methods to Avoid hard-coded example.org in test_media_handle_upload_uses_post_parent_for_directory_date() and fix a "false positive" assertion in test_media_handle_upload_ignores_page_parent_for_directory_date()

This should probably have been better served as two tickets, but I don't have the trac capability to fix that for this ticket.

So I adjust the ticket's title a little bit, instead.

#3 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 5.0

#4 @SergeyBiryukov
7 years ago

In 42742:

Tests: Avoid hardcoded domain name in test_media_handle_upload_uses_post_parent_for_directory_date().

Props birgire.
See #42736.

#5 @SergeyBiryukov
7 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 42743:

Tests: Correct a "false positive" assertion in test_media_handle_upload_ignores_page_parent_for_directory_date().

Props birgire.
Fixes #42736.

#6 @jorbin
6 years ago

  • Milestone changed from 5.0 to 5.1
Note: See TracTickets for help on using tickets.