Skip to:
Content

Opened 4 years ago

Last modified 20 months ago

#2782 assigned task

Make PHPUnit tests work, and make them dependent on BuddyPress

Reported by: boonebgorges Owned by: netweb
Milestone: Under Consideration Priority: normal
Severity: normal Version:
Component: Tools Keywords: has-patch
Cc:

Description

I'm seeing a recurrence of #2327 on a client site, and I don't understand bbPress well enough to figure out what's going on without writing some unit tests, but it's not currently possible, so here you go :)

The attached patch does the following:

  • Fixes the bbPress test bootstrap so that it actually loads bbPress
  • Fixes the bbPress testcases so that it properly installs the (empty) BBP_UnitTest_Factory
  • Provides a bare-bones version of BBP_UnitTest_Factory_For_Forum. This is all I needed to debug my specific issue, but you get the idea.
  • Detects the presence of a BuddyPress checkout, and installs + bootstraps it if found
  • Loads the BP factory into the BBP_UnitTestCase object

The patch also includes a couple of tests and a suggested fix for #2327. It looks like the basic problem is bbPress's initial setting of group forum status depends on the forum's having been created inside of the group interface. Forums that are created in any other way - programatically, in my client's case - don't seem to have the proper value set. In the long run, you might want to break this logic out from your BP_Group_Extension class and put it somewhere where it'll be enforced in a stricter way. (IMHO, BP_Group_Extension classes work best when used only as view/controllers. When used for business logic, you get problems like this one. This is a lesson I have learned the hard way on multiple occasions :) ) Anyway, my suggested fix is airtight, though perhaps it's inefficient. You can take or leave it.

Regardless of the #2327 fix, I hope you'll consider adopting the unit test framework, as I personally would be more willing to dive into some of the gnarlier bugs I occassionally come across in bbPress if it were possible to attack them in a TDD fashion.

Attachments (4)

2782.diff (6.8 KB) - added by boonebgorges 4 years ago.
2782.2.diff (7.4 KB) - added by netweb 4 years ago.
2782.3.diff (2.9 KB) - added by netweb 4 years ago.
2782.4.diff (3.3 KB) - added by boonebgorges 4 years ago.

Download all attachments as: .zip

Change History (21)

@boonebgorges
4 years ago

#1 @solhuebner
4 years ago

+1 for adopting the unit testing framework

#2 @netweb
4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.6
  • Owner set to netweb
  • Status changed from new to assigned

Thanks Boone, this is greatly appreciated and might just be that helping hand I needed to finally get my head wrapped around all things PHPUnit :)

@netweb
4 years ago

#3 @netweb
4 years ago

In 5672:

Exclude BuddyPress tests when running phpunit

BuddyPress unit testing will conflict with stand alone bbPress user testing, when BuddyPress is active the core members component cannot be deactivated thus testing users with BuddyPress at /member/username vs bbPress only /user/username and custom rewrites /custom-user-slug/username will cause conflicts in user PHPUnit tests.

A new Grunt sub task grunt phpunit:buddypress has been added to test BuddyPress integration with bbPress without compromising bbPress standalone PHPUnit testing.

Props netweb. See #2782

#4 @netweb
4 years ago

In 5673:

Add support for BuddyPress PHPUnit test integration

  • Bootstrap and load /src/bbpress.php when loading the test environment
  • Detect and load BuddyPress when running the BuddyPress PHPUnit tests
  • Setup BP_UnitTest_Factory in BBP_UnitTestCase during new BuddyPress PHPUnit test integration
  • Bootstrap and load new factory class BBP_UnitTest_Factory_For_Forum for creating forums

Props boonebgorges. See #2782

@netweb
4 years ago

#5 @netweb
4 years ago

Boone,

Hopefully the couple of commit messages above make sense in what I've done thus far :)

There are two remaining pieces here not yet committed from your original patch and these are now in 2782.3.diff which are the BP groups unit tests and the proposed fix.

Reason being is the that the first unit test: test_bbp_is_forum_public_should_be_true_for_public_group_forums() is not creating a public BP group as I would expect it to be and the assertion is failing, no matter what status you change this to the test always pass's, e.g. somethingoutherthanpublic the test still pass's.

The remaining two tests in test_bbp_is_forum_public_should_be_false_for_private_group_forums() for private and test_bbp_is_forum_public_should_be_false_for_hidden_group_forums() for hidden pass as expected.

It's late here so I'm done for the night but will take a closer look tomorrow at both the tests and the issue.

#6 @boonebgorges
4 years ago

netweb - Thanks for setting this up!

I can't reproduce the behavior you've described here, though it may be that I've misunderstood what you're suggesting. With the tests in 2782.3.diff but WITHOUT the /src/ patch, I run grunt phpunit:buddypress. The 'private' and 'hidden' tests fail. WITH the /src/ patch, the tests pass.

In 2782.4.diff, I added assertions to show that the groups are indeed being created with the declared statuses.

@boonebgorges
4 years ago

#7 @netweb
4 years ago

Thanks, that clears it up... I was testing the BP group status, in the .3 patch, I had changed it from public to yolostatus and it still passed the test even though it is not a valid BP group status, the added assertions in the .4 adds that extra check which is a good thing :)

#8 @boonebgorges
4 years ago

I assume that when you set 'yolostatus', BP was rejecting it as an invalid status, and setting it to 'public' instead, which would explain why the test passed.

#9 @netweb
4 years ago

It was setting it null, with the most recent patch, 2782.4.diff setting both values to yolostatus the assertion fails with null != yolostatus, either way we're all good, I'm now just writing more unit tests for public, private, and hidden forums, there is more going on here in regard to the original background issue, so when in Rome as they say ;)

#10 @netweb
4 years ago

In 5699:

Tests: Travis CI Improvements

  • Brings .travis.yml configuration up to speed with BuddyPress'
  • Preliminary support for side-by-side BuddyPress testing integration

Props netweb. See #2782

This ticket was mentioned in Slack in #bbpress by netweb. View the logs.


4 years ago

This ticket was mentioned in Slack in #bbpress by netweb. View the logs.


4 years ago

#13 @johnjamesjacoby
4 years ago

Nice work everyone. What's left to do here, incase someone else wants to jump in?

#14 @netweb
3 years ago

Missed the ticket in the commit, see [6031]

Tests: In PHPUnit bootstrap check if the BP_TESTS_DIR constant is first defined before attempting to load BuddyPress PHPunit testcase.php

#15 @netweb
3 years ago

I think there are still a couple of issues in bootstrapping BuddyPress alonside bbPress here, need to verfiy this and add 2782.4.diff and 2794.02.tests.patch, see also: https://codex.buddypress.org/developer/automated-testing/writing-automated-tests-for-buddypress-dependent-plugins/

This ticket was mentioned in Slack in #bbpress by jjj. View the logs.


2 years ago

#17 @johnjamesjacoby
20 months ago

  • Milestone changed from 2.6 to Under Consideration
  • Type changed from defect to task

This can be done in any milestone, so let's move this out of 2.6.

Note: See TracTickets for help on using tickets.