![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#35987 closed enhancement (fixed)
Unit test: wp_ext2type
| Reported by: | borgesbruno | Owned by: | boonebgorges |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.6 |
| Component: | Media | Version: | |
| Severity: | normal | Keywords: | has-unit-tests has-patch |
| Cc: | Focuses: |
Description
I wrote tests for the method wp_ext2type to improve the code coverage, there weren't no tests for this method.
Attachments (5)
Change History (17)
#3
@
10 years ago
- Keywords needs-patch added; has-patch has-unit-tests removed
- Milestone Awaiting Review → Future Release
Thanks for the patch.
Your tests perform spot-checks only. But I'm thinking that this is a case where we may want to refactor slightly to allow for complete test coverage. See, eg, wp_get_mime_types(), which is just a wrapper for a filtered array. How about introducing wp_get_ext_types(), moving the array there, and then using it as a dataProvider? We can then also have targeted tests for the peculiar behavior of in_array(), such as the 'JPG' =>
#4
@
10 years ago
Actually, this is a such great idea @boonebgorges, I going to do that.
Tks for the review!
#5
@
10 years ago
- Keywords has-patch has-unit-tests added; needs-patch removed
@boonebgorges I made the changes that you said, I didn't test each extension in the array provider, because using just one as sample is better for performance and covers the whole logic.
Let me know if I get your point =)
#6
@
10 years ago
- Keywords needs-patch added; has-patch removed
@borgesbruno Thanks for the updated patch. A few quick bits of feedback:
- The
ext2typefilter has gone missing. - Can you combine the functional change and the unit tests into one file? This makes testing easier.
- When you upload a new patch, you can use the same file name (
35987.diff) each time and Trac will automatically rename it for you.
#7
@
10 years ago
- Keywords has-patch added; needs-patch removed
Hi again @johnbillion , tks for the great tips, Its done, following all your requests ;-)
This is the file = 35987.2.diff
#8
@
10 years ago
- Keywords 4.6-early added
- Owner set to
- Status new → assigned
Thanks, @borgesbruno! I think we should go ahead and run tests against every single extension - for such a small number, the performance implications will probably be too small to measure.
#9
@
10 years ago
Tks for the feedback @boonebgorges, it's done!
I made the tests for each extension and just for curiosity I also made a benchmark:
The patch 2
Time: 12.68 seconds, Memory: 122.25Mb
OK (1 test, 19 assertions)
The patch 3
Time: 14.19 seconds, Memory: 122.25Mb
OK (1 test, 197 assertions)
As you can see you were right, just increased one second and half with almost two hundred assertions, running in an isolate mode.
Patch, tests for wp_ext2type