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

Replace Peridot/Leo/WP_Mock tests with Kahlan #31

Merged
merged 2 commits into from
Sep 23, 2020
Merged

Conversation

mallorydxw
Copy link
Contributor

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

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 mallorydxw requested a review from RobjS September 11, 2020 18:24
This was referenced Sep 11, 2020
Copy link
Contributor

@RobjS RobjS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me 👍

There is one opportunity to tighten up the tests of register() methods if we like (and would fix something that always bugged me about WP_Mock and the looseness of expectActionAdded()).

Currently, the following would pass:

public function register()
    {
        add_action('wp_footer', [$this, 'wpFooter']);
        add_action('wp_footerx', [$this, 'wpFooter']);
    }

Tested by:

describe('->register()', function () {
        it('registers actions', function () {
            allow('add_action')->toBeCalled();
            expect('add_action')->toBeCalled()->once()->with('wp_footer', [$this->analytics, 'wpFooter']);
            $this->analytics->register();
        });
    });

However, this test wouldn't pass:

describe('->register()', function () {
        it('registers actions', function () {
            allow('add_action')->toBeCalled();
            expect('add_action')->toBeCalled()->once();
            expect('add_action')->toBeCalled()->with('wp_footer', [$this->analytics, 'wpFooter']);
            $this->analytics->register();
        });
    });

The difference between the effect of those two register() methods could be quite profound, so I feel like that could be a change worth making; what do you think?

@mallorydxw
Copy link
Contributor Author

@RobjS Thank you. I didn't notice that that's how Kahlan's allow()/expect() worked.

@RobjS
Copy link
Contributor

RobjS commented Sep 23, 2020

@mallorydxw I only worked it out by trial and error, the documentation doesn't really make it clear!

I guess it makes sense in that you wouldn't normally care about the exact number of times global functions were being called with exactly what arguments when unit testing, only that the tested method's output was as expected. But in the case of register() functions, all we care about is the calls to the global functions.

@mallorydxw mallorydxw merged commit 22c47d4 into main Sep 23, 2020
@mallorydxw mallorydxw deleted the feature/use-kahlan branch September 23, 2020 15:58
@RobjS RobjS mentioned this pull request Jan 5, 2021
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