Make WordPress Core

Opened 10 years ago

Closed 7 years ago

#26999 closed enhancement (invalid)

Improve the layout of phpunit unit tests to gauge code coverage

Reported by: wonderboymusic's profile wonderboymusic Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-patch needs-unit-tests
Focuses: Cc:


Some groups of unit tests are laid out better than others. To improve code coverage, we should be able to map files or features to a defined set of files/folders. Sure, we have "post" and "query" and whatnot, but the overlap between the 2 makes it impossible to understand what "should" go in each file. Every time I write a unit test, I am placing the new tests in a best-guess (or, most of the time, arbitrary) location.

This is a spike ticket for cleanup and improvements. If this picks up steam from me or any others, might move to 3.9.

Attachments (2)

patch.diff (46.9 KB) - added by sgrant 9 years ago.
Move two test classes to wp-include, add @covers notation
patch_formatting.diff (104.0 KB) - added by sgrant 9 years ago.
Partial refactoring of the formatting directory in tests

Download all attachments as: .zip

Change History (18)

#1 @bpetty
10 years ago

Generally speaking, I figured we were just headed down the road of organizing them by the same new component tree we have in Trac. They are mainly organized like this already, though you're right, they still need a lot more cleanup. It's already very difficult to traverse the code history on unit tests since it was merged from the unit-tests repo, we should try to avoid moving mass amounts of code even more.

As far as figuring out what level of coverage there is, that's what code coverage tools are for. We don't necessarily need to break the unit tests out to a one-to-one file match based on how the core code is organized.

#2 @jorbin
10 years ago

  • Summary changed from Improve the layout of unit tests to gauge code coverage to Improve the layout of phpunit unit tests to gauge code coverage

#3 @jorbin
10 years ago

Changed the title to make it clear that we are talking about the phpunit tests and not the qunit tests

#4 @johnbillion
10 years ago

  • Version trunk deleted

#6 @wonderboymusic
10 years ago

In 28625:

@uses means something entirely different for unit tests. More:

Code coverage analysis takes forever to run, and these annotations will cause it to fail in the middle.

See #26999.

#7 @wonderboymusic
10 years ago

In 28635:

Add more test coverage for wpdb.

See #26999.

This ticket was mentioned in IRC in #wordpress-dev by boonebgorges. View the logs.

10 years ago

#9 @boonebgorges
10 years ago

DrewAPicture brings up the @covers annotation in IRC:

We should still strive for decent organization for other reasons, but I think that @covers is probably the best way for us to gauge coverage.

#10 @sgrant
9 years ago

What about refactoring the unit test file layout to match the phpunit filesystem recommendation?

I'm also finding it difficult in some cases, especially as a relatively new contributor, to figure out the best location for new tests. Would there be a benefit to restructuring the suite to match the layout of the source it's testing? (see attached patch for simple examples that add @covers notation and move to a wp-* layout; the real benefit might be taking something like the formatting directory and organizing tests by source file instead of by ticket/test concept).

9 years ago

Move two test classes to wp-include, add @covers notation

#11 @sgrant
9 years ago

Pinging again with a few more refactors in the formatting directory. This maps tests to the source files, which seems like a more logical way to understand where a new test should go, and with @covers annotation, should make it easier to gauge coverage.

This doesn't change any behaviour, of course--it just moves tests around based on the source code they're testing, while still retaining the @group notation.

9 years ago

Partial refactoring of the formatting directory in tests

#12 @boonebgorges
9 years ago

In 31230:

Move wp_get_object_terms() tests into their own file.

See #26999.

#13 @sgrant
9 years ago

I took a crack at refactoring the structure in relation to the source (what a great way to learn about WP history!)

This layout seems easier to understand (at least to me), and it seems clear where new tests could be added in the future. However, there are a lot of test classes that don't map well to files; they test functionality instead of individual functions. For example, the directories with multiple files often have classes that define specific setUp and tearDown functions, and pulling all of those into a single class for the file in question might bloat the pre/post work.

Also, many of the tests that were just dumped in general could be refactored out to show coverage.

There are still a few small issues in the repo, but it's one possible way to consider the tests.

#14 @boonebgorges
9 years ago

In 31286:

Improve organiation of tax_query and meta_query unit tests.

meta_query tests have been moved to tests/phpunit/tests/query/metaQuery.php and tax_query tests to tests/phpunit/tests/query/taxQuery.php. This is an improvement because (a) it better corresponds to the way that other WP_Query parameter tests are organized, (b) splitting meta and tax tests into separate classes simplifies the required @group annotations, and (c) the tests have nothing to do with posts per se, and so do not belong in the post subdirectory.

The tests previously found at tests/phpunit/tests/query/taxQuery.php have been moved to isTerm.php in the same directory. These tests are related to the is_* functions that have to do with taxonomy terms, like is_category().

See #26999.

#15 @pbearne
8 years ago

This is ticket has around a long time without any action.

Suggestion: let's make it requirement that all new PHPUnit tests added to core have to include @covers and @uses

I have created a ticket to hold all the updates to the existing tests

#16 @wonderboymusic
7 years ago

  • Milestone Future Release deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.