Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 20 months ago

#6009 closed enhancement (maybelater)

Share fixtures across unit tests where appropriate

Reported by: boonebgorges Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: trac-tidy-2018
Cc:

Description

There are a few places where our unit tests could benefit hugely from sharing fixtures (ie the dummy data) across a series of tests. Take the suggestions group, for instance: these tests create a bunch of data during each setUp(), but the data is the same between tests, and is not modified during the tests. So it's safe to share it, and shave a minute or more off of the running time of our suite.

Sharing fixtures should generally be done with caution, but when the tests are testing only *get* functionality, it should be safe.

Change History (11)

#1 @boonebgorges
5 years ago

There are some complications to making setUpBeforeClass() work in our suite. This method (and tearDownAfterClass() must be called statically. This means that they must also call all testcase and factory methods statically. So the relevant methods will have to be defined as static. This in itself is not too big a deal, but create_user() in particular causes problems, because when static, a new factory will have to be defined for each call; and when that happens, the WP_UnitTest_Generator_Sequence objects are not persistent.

For this reason, create_user() should be removed, and we should move that logic into our own BP_UnitTest_Factory_For_User which will extend the WP version to do our specific BP work. This has the added benefit of greater consistency in our tests.

#2 @boonebgorges
5 years ago

In 9139:

In automated tests, move user creation to a proper factory method.

BP user creation requires that a couple of extra pieces of data be set up
(last activity, display name, etc). So we previously had a wrapper in
BP_UnitTestCase called create_user() that performed the extra setup.
However, the wrapper made it impossible to use create_user() statically,
because the user_login and user_email iterator was not persistent from call to
call. (The create_user() syntax is also a break with the rest of our unit
tests, which is not ideal.)

This changeset introduces BP_UnitTest_Factory_For_User, which reproduces the
customizations of create_user(), but in a proper factory method. All
instances of create_user() throughout the test suite have also been replaced.

See #6009.

#3 @boonebgorges
5 years ago

In 9140:

Move automated test mail handlers to setUpBeforeClass().

In many cases, creating data in BuddyPress (such as friendships) will generate
an email notification. In order to avoid errors when generating fixtures, our
automated suite places some filters on 'wp_mail'. However, these filters were
previously added during setUp(), which is too late for test classes that
create fixtures during setUpBeforeClass().

See #6009.

#4 @boonebgorges
5 years ago

In 9141:

Make BP_UnitTestCase::set_current_user() a static method.

This allows it to be called from outside of an object context.

See #6009.

#5 @boonebgorges
5 years ago

In 9142:

Share fixtures across Suggestions tests.

This improves the running time of our automated test suite by almost a minute.

See #6009.

#6 @DJPaul
5 years ago

  • Milestone changed from 2.2 to Future Release

#7 @boonebgorges
5 years ago

In 9561:

Don't create a current user by default in most unit tests.

The default state of our tests should be as a logged-out user. BP does pretty
different things with different kinds of logged-in users, so individual tests
should decide for themselves what the current user status should be.

Moreover, the creation of fixture users is fairly resource-intensive. Not
creating users when they're not needed makes the test suite run 5-10% faster.

See #6009.

#8 @boonebgorges
5 years ago

In 9562:

Ensure a current user is set in test_total_group_count_groups_accept_membership_request().

In versions of WP prior to 4.1, the failure to have a current user was causing
a PHP notice to be thrown.

See #6009.

#9 @DJPaul
3 years ago

  • Type changed from task to enhancement

#10 @DJPaul
20 months ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#11 @DJPaul
20 months ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.