WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#35492 closed enhancement (fixed)

Separate unit test factory classes into separate files

Reported by: ericlewis Owned by:
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

Similar to the spirit of #33413, I suggest we split unit test factory classes into their own files for readability purposes.

Attachments (2)

35492.diff (135.6 KB) - added by ericlewis 4 years ago.
35492.2.diff (3.2 KB) - added by ericlewis 4 years ago.

Download all attachments as: .zip

Change History (11)

#1 @swissspidy
4 years ago

  • Component changed from General to Build/Test Tools
  • Keywords needs-patch added

I found that quite annoying when I had to look up some methods of the factories, so +1.

#2 @netweb
4 years ago

Related: #30017

#3 follow-up: @ericlewis
4 years ago

  • Keywords needs-patch removed

As this will be a somewhat large change, I'd like to propose the solution before putting together a patch.

Split out classes into a new folder tests/phpunit/includes/factory/, which will include

class-wp-unittest-factory-for-post.php
class-wp-unittest-factory-for-attachment.php
class-wp-unittest-factory-for-user.php
class-wp-unittest-factory-for-comment.php
class-wp-unittest-factory-for-blog.php
class-wp-unittest-factory-for-network.php
class-wp-unittest-factory-for-term.php
class-wp-unittest-factory-for-thing.php
class-wp-unittest-generator-sequence.php
class-wp-unittest-factory-callback-after-create.php

then require_once each of these in `bootstrap.php`.

Does that sound good?

Last edited 4 years ago by ericlewis (previous) (diff)

#4 @boonebgorges
4 years ago

  • Keywords dev-feedback 2nd-opinion removed

Plan sounds good to me.

When creating the new files, make sure to use svn cp so that we don't lose revision history.

@ericlewis
4 years ago

#5 in reply to: ↑ 3 @ericlewis
4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.5

Replying to ericlewis:

[The plan is to split] out classes into a new folder [...] then require_once each of these in `bootstrap.php`.

Actually, for backwards compatibility, we should require_once these in factory.php, where I've left the main class WP_UnitTest_Factory.

Proposed patch in attachment:35492.diff.

#6 @ericlewis
4 years ago

In 36347:

Build/Test Tools: Move PHP factory classes into their own files.

This makes the code easier to browse.

factory.php loads the new files, so this is backwards compatible in case factory.php is loaded directly for access to one of the classes.

See #35492.

#7 follow-up: @ericlewis
4 years ago

  • Keywords has-patch removed

Should we move class WP_UnitTest_Factory into its own file as well?

#8 in reply to: ↑ 7 @ericlewis
4 years ago

Replying to ericlewis:

Should we move class WP_UnitTest_Factory into its own file as well?

Yes, I think so.

@ericlewis
4 years ago

#9 @ericlewis
4 years ago

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

Missed the ticket, r36409 moves class WP_UnitTest_Factory into its own file.

Note: See TracTickets for help on using tickets.