Opened 10 years ago
Last modified 8 years ago
#2782 assigned task (blessed)
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)
Change History (21)
#2
@
10 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 :)
#5
@
10 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
@
10 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.
#7
@
10 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
@
10 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
@
10 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 ;)
This ticket was mentioned in Slack in #bbpress by netweb. View the logs.
10 years ago
This ticket was mentioned in Slack in #bbpress by netweb. View the logs.
10 years ago
#14
@
9 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
@
8 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/
+1 for adopting the unit testing framework