Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making FilterWPQuery Reusable, Swappable, & Flexible #5

Merged
merged 12 commits into from May 27, 2018
Merged

Making FilterWPQuery Reusable, Swappable, & Flexible #5

merged 12 commits into from May 27, 2018

Conversation

hellofromtonya
Copy link
Collaborator

@hellofromtonya hellofromtonya commented May 11, 2018

This pull request separates the task of "getting the content" from FilterWPQuery. It makes the class reusable while providing a mechanism to wire up specific project implementations for getting content for the search query.

It's built off of PR #4. Therefore, the commits for this PR begin at commit d08fac0.

Improvements include:

  • Added a repo for content getters
  • Added a contract to strictly define how to interface with each implementation
  • Moved the posts generator into its own implementation
  • Added an init() method to FilterWPQuery and injected the implementation.
  • Modified getPosts() to use the generic object and invoke the getContent method on whatever implementation is wired to it.

With this change, FilterWPQuery does not care which implementation is used. It provides a clean way to reuse this code from project-to-project.

This PR is Part 4 of my article series on Torque Magazine.

This interface is a contract that strictly defines how to interact
with each "getting the content" implementation.

We can use this contract for type hinting when we inject an
implementation into FilterWPQuery.
Notice that this implementation implements the ContentGetterContract.
Therefore, it has the standard `getContent` method, which will be
invoked by FilterWPQuery.
Notice how getPosts() uses the new generic $contentGetter object
to invoke through the contract to the specific implementation that
is bound to the $contentGetter property.
Notice that we are type hinting the injected object using the
contract.  This contract serves an interface between the FilterWPQuery
and each implementation.

Using this design, the FilterWPQuery class does not care which
implementation is wired to it.  Rather, it serves as a generic object.
But because of the contract, the FilterWPQuery knows how to interface
with it.
During the launch, we initialize the FilterWPQuery by injecting
which implementation we want for our project.
@Shelob9
Copy link
Contributor

Shelob9 commented May 26, 2018

Travis is failing with Fatal error: Uncaught Error: Call to undefined method PHPUnit\Util\Configuration::getExtensionConfiguration() in /app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php on line 908 Error: Call to undefined method PHPUnit\Util\Configuration::getExtensionConfiguration() in /app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php on line 908

@hellofromtonya
Copy link
Collaborator Author

@Shelob9 I'll take a look and resolve it.

@Shelob9
Copy link
Contributor

Shelob9 commented May 26, 2018

Running tests locally:

There were 3 errors:

1) CalderaLearn\RestSearch\Tests\Integration\FilterWPQueryTest::testGetPosts
Error: Call to a member function getContent() on null

/app/src/FilterWPQuery.php:103
/app/Tests/Integration/FilterWPQueryTest.php:79
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:708
/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:508

2) CalderaLearn\RestSearch\Tests\Integration\FilterWPQueryTest::testGetPostsArePosts
Error: Call to a member function getContent() on null

/app/src/FilterWPQuery.php:103
/app/Tests/Integration/FilterWPQueryTest.php:92
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:708
/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:508

3) CalderaLearn\RestSearch\Tests\Integration\RestRequestTest::testFilteringRESTRequest
Error: Call to a member function getContent() on null

/app/src/FilterWPQuery.php:103
/app/src/FilterWPQuery.php:59
/tmp/wordpress/wp-includes/class-wp-hook.php:288
/tmp/wordpress/wp-includes/plugin.php:244
/tmp/wordpress/wp-includes/class-wp-query.php:2756
/tmp/wordpress/wp-includes/class-wp-query.php:3230
/tmp/wordpress/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php:291
/tmp/wordpress/wp-includes/rest-api/class-wp-rest-server.php:936
/tmp/wordpress-tests-lib/includes/spy-rest-server.php:46
/app/Tests/Integration/RestRequestTest.php:42
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:708
/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:508

ERRORS!
Tests: 9, Assertions: 15, Errors: 3.
Script docker-compose run --rm wordpress_phpunit phpunit --configuration phpunit-integration.xml.dist handling the wp-tests event returned with error code 2

@Shelob9
Copy link
Contributor

Shelob9 commented May 26, 2018

I didn't realize I was pushing into your branch. We can reset that into its own if the idea I had in the last two commits don't work out.

I made

BTW I added a mock \WP_Query 2a3e1d6#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780R39 but that should make the existing tests pass, but isn't useful. More reason for Tonya to write about using Mockery.

Also, tests are not working, same reasons as above. Let's get those sorted and then finish updating the interfaces. Then a factory to glue them all together.

Also an interface for ModifyQueryArgs than a factory for all related objects.

@hellofromtonya
Copy link
Collaborator Author

@Shelob9 To keep this PR clean and synchronized with the Torque code review article, I'm going to rollback last changes and force push. Then we can create a new branch and PR to build out the Factory and glue it all together, per our discussions.

We'll use Brain Monkey in the unit tests.

Mockery we'll use in both test suites.
The new FilterWPQuery requires an implementation for getting the posts.
This commit mocks the interface and then loads it via the `init()` method.

In addition, switched to using Mockery and Brain Monkey instead of a separate
mocked stub.
WP_Post mock begin autoloaded can cause the integration tests to fail.  Load it only if needed.

With the latest version of PHPUnit, a fatal error is thrown in WP Core Test Suite:

Class PHPUnit_Util_Test may not inherit from final class (PHPUnit\Util\Test) in /tmp/wordpress-tests-lib/includes/phpunit6-compat.php on line 18

Downgraded to version 5.7.9 to resolve the issue.
1. Whoops, forgot to import the classes.
2. The private method is on the object and not static.
Before we can go get the posts, first we need to initialize FilterWPQuery by
loading the implementation.
@hellofromtonya hellofromtonya merged commit 27d780d into caldera-learn:master May 27, 2018
@hellofromtonya hellofromtonya deleted the torque/code-review/part4 branch May 27, 2018 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants