Opened 11 months ago
Closed 8 months ago
#56203 closed task (blessed) (fixed)
Reduce code duplication in AJAX attachment handling tests
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | minor | Version: | 6.1 |
Component: | Media | Keywords: | has-patch |
Focuses: | Cc: |
Description
The tests for AJAX attachment handling contain duplicated code to preform the following tasks.
a) create a new admin user and set them as the current user
https://github.com/WordPress/wordpress-develop/blob/6.0.0/tests/phpunit/tests/ajax/Attachments.php#L17-L27
b) upload selected file and create an attachment
https://github.com/WordPress/wordpress-develop/blob/6.0.0/tests/phpunit/tests/ajax/Attachments.php#L29-L33
It would be great to replace the duplicated code with some helper functions. It would make creating more tests simpler.
Attachments (1)
Change History (11)
#2
@
11 months ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Awaiting Review to 6.1
#4
@
11 months ago
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
Hi there, thanks for the ticket and the patch! The first part is now committed.
The attachments part looks good too, but it appears that the same pattern is also used in other tests, not only Ajax. So I think this 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.
#5
@
11 months ago
Thanks for committing the first part, @SergeyBiryukov.
I created a new ticket for the helper method handling file upload and attachment creation. Patch will be ready soon.
#6
follow-up:
↓ 10
@
9 months ago
What's next for this ticket, @SergeyBiryukov? Can we close it since the changes are already committed? There is also a follow-up ticket (56248) that's ready for review.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
9 months ago
#8
@
9 months ago
- Type changed from enhancement to task (blessed)
As discussed ahead of today's scheduled 6.1 Beta 1 release party (see the Slack link above), it was deemed that this ticket was now a test-only ticket and could be converted to a task for additional follow up in the remaining 6.1 cycle.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
9 months ago
#10
in reply to:
↑ 6
@
8 months ago
- Resolution set to fixed
- Status changed from reviewing to closed
Replying to martin.krcho:
Can we close it since the changes are already committed? There is also a follow-up ticket (#56248) that's ready for review.
Ah, I was going to circle back here for comment:4, but I see there is now a separate ticket for that, so let's continue there. This one can be closed. Thanks!
Patch reducing the duplication in the AJAX attachments handler tests