Opened 2 years ago
Last modified 22 months ago
#56248 reviewing enhancement
Reduce code duplication in tests that upload file as attachment
Reported by: | martin.krcho | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | minor | Version: | 6.1 |
Component: | Media | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Couple of unit tests contain code to upload a file and make it an attachment. These could be simplified by removing some of the duplication.
Example of the duplicated code
$filename = DIR_TESTDATA . '/images/canola.jpg'; $contents = file_get_contents( $filename ); $upload = wp_upload_bits( wp_basename( $filename ), null, $contents ); $attachment = $this->_make_attachment( $upload );
As already discussed in 56203, the solution can be a general helper method in WP_UnitTestCase_Base
, next to the ::_make_attachment()
method. That way it can also be reused in other tests. The name would probably be _upload_file_and_make_attachment()
for consistency.
Attachments (1)
Change History (16)
This ticket was mentioned in PR #3102 on WordPress/wordpress-develop by martinkrcho.
2 years ago
#3
- Keywords has-unit-tests added
The PR introduces a new method WP_UnitTestCase_Base::_upload_file_and_make_attachment()
that uploads given file and creates an attachment post from it.
It also replaces all occurrences of the repetitive code - with the exception of few places where extra tests are ran to check the result of wp_upload_bits()
call.
Trac ticket: https://core.trac.wordpress.org/ticket/56248
This ticket was mentioned in Slack in #core by martin.krcho. View the logs.
2 years ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
2 years ago
#6
@
2 years ago
- Milestone changed from Awaiting Review to 6.2
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
This ticket was mentioned in Slack in #core by costdev. View the logs.
23 months ago
#8
@
23 months ago
I like the idea of consolidating code into a reusable function for all of the tests that need it. Not sure though if putting it in the Base abstract is the right place for it. A trait may be better. That way each test that needs it can use that trait to gain access. Adding to my list to review the PR and think about how to encapsulate it for reuse.
#9
@
23 months ago
I like the idea of using a trait instead of adding the function _upload_file_and_make_attachment
to the base class. I'll try to find some time to refactor it.
#11
@
23 months ago
The PR is updated.
_upload_file_and_make_attachment()
and _upload_file()
were moved to a new trait.
I also updated the PHPdocs where needed.
#12
@
23 months ago
- Keywords changes-requested removed
Looks like a solid patch, I like the refactoring as well, it's more efficient like that, thank @martinkrcho
@martin.krcho commented on PR #3102:
23 months ago
#13
@hellofromtonya I accidentally removed you from the reviewers. Anyway, the code is refactored to use a trait now and ready for another review.
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
#15
@
22 months ago
- Milestone changed from 6.2 to Future Release
This ticket was discussed during the bug scrub. As this ticket still needs some work and 6.2 Beta 1 is being released today, I'll move this to Future Release
.
As this is a tests-only ticket, it can still be committed during this cycle, so if the patch is deemed ready, the ticket can be pulled back into the 6.2 milestone for commit
.
Additional props: @mukesh27
The attached patch introduces a new method
WP_UnitTestCase_Base::_upload_file_and_make_attachment()
that uploads given file and creates an attachment post from it.The patch also replaces all occurrences of the repetitive code - with the exception of few places where extra tests are ran to check the result of
wp_upload_bits()
call.