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

Remove WP_Mock #21

Closed
wants to merge 3 commits into from
Closed

Remove WP_Mock #21

wants to merge 3 commits into from

Conversation

mallorydxw
Copy link
Contributor

@mallorydxw mallorydxw commented Aug 18, 2020

I think this library has a dependency on an older version of PHPUnit, and some other libraries we're using have dependencies on newer versions of PHPUnit. This is leading to problems. So I want to remove WP_Mock and replace it with PHPMockery.

All new projects will be running on Saluki/Dalmatian. So 7.4 is the only
version we care about right now.
@mallorydxw mallorydxw force-pushed the feature/remove-wp-mock branch from ff00fce to dd477ad Compare August 21, 2020 13:21
- composer remove --dev 10up/wp_mock
- composer require --dev php-mock/php-mock-mockery
- Manually replaced WP_Mock with PHPMockery

WP_Mock has dependencies on older versions of packages. This ends up
conflicting with the newer versions of more actively-developed packages.
Removing this dependency allows us greater flexibility in the packages
we install.
@mallorydxw mallorydxw force-pushed the feature/remove-wp-mock branch from dd477ad to 8b34a50 Compare August 21, 2020 13:24
@mallorydxw mallorydxw marked this pull request as ready for review August 21, 2020 13:25
@RobjS
Copy link
Contributor

RobjS commented Aug 21, 2020

I'd be very reluctant to lose WP_Mock. I know it's just syntactical sugar for things that could be done without it, but I find the PHPMockery approach hard to read at times.

The composer issues I'm seeing in PHP 7.4 arise from outdated dependencies in Peridot, which is specifying "phpunit/php-timer": "^1",, which is incompatible with WP_Mock's (fairly loose) requirement for "phpunit/phpunit" : ">=7.0", in its latest version (0.4.2).

It looks like Peridot hasn't had a release in > 3 years: https://github.com/peridot-php/peridot/tags.

@RobjS
Copy link
Contributor

RobjS commented Aug 21, 2020

Maybe https://github.com/kahlan/kahlan would be a potential Peridot alternative? Peridot is described as "basically a lite version of kahlan with a smaller community and resources behind it" here: peridot-php/peridot#214

@mallorydxw
Copy link
Contributor Author

I'd be very reluctant to lose WP_Mock. I know it's just syntactical sugar for things that could be done without it, but I find the PHPMockery approach hard to read at times.

PHPMockery is just a thin wrapper around Mockery - and we use Mockery whenever we need to mock a class. So personally, I feel like using PHPMockery + Mockery, I'm just keeping one mocking library in my head rather than two.

The composer issues I'm seeing in PHP 7.4 arise from outdated dependencies in Peridot, which is specifying "phpunit/php-timer": "^1",, which is incompatible with WP_Mock's (fairly loose) requirement for "phpunit/phpunit" : ">=7.0", in its latest version (0.4.2).

It looks like Peridot hasn't had a release in > 3 years: https://github.com/peridot-php/peridot/tags.

I didn't realise that. Yes, very happy to replace Peridot + Leo with other things.

But I do think WP_Mock is stopping us from installing recent versions of psalm:

On branch chore/php-7.4 (sebastian/diff isn't phpunit, but it's one of the libraries phpunit uses):

% composer require --dev vimeo/psalm
Using version ^3.14 for vimeo/psalm
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Conclusion: remove vimeo/psalm 3.14.1
    - Conclusion: don't install vimeo/psalm 3.14.1
    - Installation request for vimeo/psalm ^3.14 -> satisfiable by vimeo/psalm[3.14.0, 3.14.1].
    - Conclusion: don't install sebastian/diff 2.0.1
    - vimeo/psalm 3.14.0 requires sebastian/diff ^3.0 || ^4.0 -> satisfiable by sebastian/diff[4.0.2, 3.0.0, 3.0.1, 3.0.2, 4.0.0, 4.0.1].
    - Can only install one of: sebastian/diff[3.0.0, 2.0.1].
    - Can only install one of: sebastian/diff[3.0.1, 2.0.1].
    - Can only install one of: sebastian/diff[3.0.2, 2.0.1].
    - Can only install one of: sebastian/diff[2.0.1, 4.0.2].
    - Can only install one of: sebastian/diff[4.0.0, 2.0.1].
    - Can only install one of: sebastian/diff[4.0.1, 2.0.1].
    - Can only install one of: sebastian/diff[4.0.2, 2.0.1].
    - Installation request for sebastian/diff (locked at 2.0.1) -> satisfiable by sebastian/diff[2.0.1].


Installation failed, reverting ./composer.json to its original content.

On branch feature/remove-wp-mock:

% composer require --dev vimeo/psalm
Using version ^3.14 for vimeo/psalm
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Nothing to install or update
Writing lock file
Generating autoload files
composer/package-versions-deprecated: Generating version class...
composer/package-versions-deprecated: ...done generating version class
23 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> vendor/bin/phar-install
+ export TMP_DIR=/tmp/phar-install-tmp-dir
+ export TMP_FILE=/tmp/phar-install-tmp.phar
+ pwd
+ export PHAR=/workbench/src/github.com/dxw/wordpress-template/wp-content/themes/theme/vendor.phar
+ rm -rf /tmp/phar-install-tmp-dir /tmp/phar-install-tmp.phar
+ mkdir -p /tmp/phar-install-tmp-dir
+ COMPOSER_VENDOR_DIR=/tmp/phar-install-tmp-dir/vendor composer install --no-dev --no-interaction
Loading composer repositories with package information
Installing dependencies from lock file
Package operations: 5 installs, 0 updates, 0 removals
  - Installing aura/autoload (2.0.4): Loading from cache
  - Installing dxw/iguana (v1.1.1): Loading from cache
  - Installing dxw/iguana-extras (v1.0.0): Loading from cache
  - Installing dxw/iguana-theme (v1.0.0): Loading from cache
  - Installing dxw/pagination (v1.0.0): Loading from cache
Generating autoload files
+ cd /tmp/phar-install-tmp-dir
+ php -d phar.readonly=Off
Building phar file files
+ mv -f /tmp/phar-install-tmp.phar /workbench/src/github.com/dxw/wordpress-template/wp-content/themes/theme/vendor.phar
+ rm -rf /tmp/phar-install-tmp-dir /tmp/phar-install-tmp.phar

Do let me know if you're able to install psalm without removing WP_Mock somehow. Because when working with sites that use WP_Mock I was finding that I could only install an older version of psalm that didn't work with PHP 7.4.

@RobjS
Copy link
Contributor

RobjS commented Aug 21, 2020

I'm able to have Psalm & WP_Mock installed alongside one another if I remove all the peridot-php packages (peridot, leo and peridot-dot-reporter) and update WP_Mock to its latest version (0.4.2). This generates a successful install:

...
 "config": {
      "platform": {
        "php": "7.4.3"
      }
    },
    "require-dev": {
        "dxw/phar-install": "^1.0",
        "10up/wp_mock": "^0.4.2",
        "mikey179/vfsstream": "^1.6",
        "dxw/php-cs-fixer-config": "^1.0",
        "vimeo/psalm": "^3.14"
    },
...

composer require --dev kahlan/kahlan on top of this also completes successfully.

@mallorydxw
Copy link
Contributor Author

Oh, I stand corrected.

I'd still like to avoid having two mocking libraries living in my brain.

kahlan seems to have its own mocking library. What if we got rid of both peridot and WP_Mock and just used kahlan's built-in mocking?

@RobjS
Copy link
Contributor

RobjS commented Aug 24, 2020

That makes sense to me. It looks like Kahlan's in-built mocking would be enough to cover anything we'd have used WP_Mock/Mockery/PHPMockery for in the past, so if we could consolidate all those things into one that'd be good 👍

@mallorydxw mallorydxw marked this pull request as draft September 1, 2020 14:00
mallorydxw added a commit that referenced this pull request Sep 11, 2020
Per discussion on ticket #21 Rob and I agreed that replacing
Peridot/Leo/WP_Mock with Kahlan would be the best way to allow us to
update dependencies and allow installing the latest version of Psalm.

Remove peridot dependencies and config file:

- rm peridot.php
- composer remove --dev peridot-php/peridot peridot-php/leo peridot-php/peridot-dot-reporter 10up/wp_mock
- GitHub Actions: Remove peridot test

Add Kahlan dependencies and config file:

- composer require --dev kahlan/kahlan
- Add kahlan-config.php
- GitHub Actions: Add kahlan test

Leo-to-Kahlan changes:

- s/->to->be->an->instanceof/toBeAnInstanceOf/
- s/to->equal/toEqual/
- s/to->contain/toContain/
- s/to->be->empty/toBeEmpty/
- s/to->be->equal/toEqual/

WP_Mock-to-Kahlan changes:

- Remove all WP_Mock::setUp and WP_Mock::tearDown
- Convert WP_Mock::expectActionAdded to expect()
- Convert WP_Mock::wpFunction to allow()/expect()

Miscellaneous changes:

- Use namespaces in test files
- composer update

Link: #21
@mallorydxw
Copy link
Contributor Author

Closing this in favour of #31.

@mallorydxw mallorydxw closed this Sep 11, 2020
@RobjS RobjS deleted the feature/remove-wp-mock branch March 12, 2021 16:11
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.

2 participants