Brain-WP/BrainMonkey

Failing test can impact other tests

Closed this issue · 14 comments

I've not done any investigation as to why, but I've managed to produce a minimal working example which demonstrates the problem.

The following is quite contrived but consider the following test case which has two tests. The first test fails (as is expected), but in doing so, it causes the second to also fail: When the second is run in isolation it passes, when it is run after the failing test it also fails.

It fails because the value being returned is null: it's as though Monkey forgets the return value after a failure.

<?php
use Brain\Monkey;

class blogFunctionsTest extends PHPUnit_Framework_TestCase {

    public function setUp() {
        parent::setUp();
        Monkey::setUpWP();
    }

    public function tearDown() {
        Monkey::tearDownWP();
        parent::tearDown();
    }

    public function testA() {

        Monkey::functions()->expect('get_blog_option')
            ->once()
            ->with( 53, 'blogname' )
            ->andReturn( 'Mr Bloggy' );

        $this->assertEquals( 'Mr Bloggy', \get_blog_option( 53, 'blogname' ) );

        //This assertion should trigger an error, failing the test
        $this->assertEquals( 'Mr Bloggy', \get_blog_option( 53, 'blogname' ) );
    }

    public function testB() {

        Monkey::functions()->expect('get_blog_option')
            ->zeroOrMoreTimes()
            ->with( 53, 'blogname' )
            ->andReturn( 'Mr Bloggy' );

        //These assertions should pass...
        $this->assertEquals( 'Mr Bloggy', \get_blog_option( 53, 'blogname' ) );
        $this->assertEquals( 'Mr Bloggy', \get_blog_option( 53, 'blogname' ) );
    }

}

Summary:

 phpunit --filter=blogFunctionsTest

Result:

There was 1 error:

1) blogFunctionsTest::testA
Mockery\Exception\InvalidCountException: Method get_blog_option(53, "blogname") from Mockery_0__get_blog_option should be called
 exactly 1 times but called 2 times.
...

--

There was 1 failure:

1) blogFunctionsTest::testB
Failed asserting that null matches expected 'Mr Bloggy'.

FAILURES!
Tests: 2, Assertions: 3, Failures: 1, Errors: 1.

Running just the second test:

 phpunit --filter=blogFunctionsTest::testB

Result:

OK (1 test, 2 assertions)

Other observations

  • It appears to be tied to the function which is not invoked correctly. I.e. if I change the function name in the second test, it does not fail, but changing the arguments as no effect (the second test still fails).
  • As earlier mentioned, the order matters (but that is to be expected).
  • If I run an identical copy of testA() - the second test fails (as we would expect) but not because it is invoking a function twice which should only be invoked once but because the return value is null.

Thanks for reporting @stephenharris!

It seems that the Mockery expectation object for same action persists among tests, which is absolutely wrong.

I'll look at this as soon as possible.

@stephenharris I did a quick test copying the test code you provided, and I get different results than you (still buggy, but different) can you please say me the version of PHP, PHPUnit and Mockery you are using?

Thanks for the quick response @Giuseppe-Mazzapica

  • PHPUnit 4.6.4
  • PHP 5.6.8
  • Mockery 0.9.3

@stephenharris seems it has been fixed. Can you please give a try by requiring dev-master? If works for you, I'll close this and release a new version. Thanks!

I've updated but I'm getting

Call to undefined function Patchwork\undoAll()

I'm using antecedent/patchwork 1.3.5

Yes, it is defined. Can you please give a try by manually loading Patchwork.php before executing tests? Maybe in tests bootstrap if you have one...

Yes that's it:

 require 'vendor/antecedent/patchwork/Patchwork.php';

You have that in your bootstrap and function undefined? mmm... really confused, can't really understand how that is possible...

No, sorry, I meant that works now. I guess it's autoloading the functions. It hadn't been an issue before so I had skipped over that when setting up.

Well, yes, I enforced version of 1.3.* because newer versions throws warnings... But Monkey::setUp() should load Patchwork.php... but very likely is too late in your case.

It does seem odd that Monkey::setUp() is too late: I would imagine that the only functions being redefined were the ones being mocked, and they aren't being mocked until after Monkey::setUp() is called. I'll do a little digging, but i'm happy for this issue to be closed. Thanks for your help.

@stephenharris Yes, Monkey::setUp() should be fine, but I had this issue other times, I guess that is something related to autoload...

In documentation I wrote that in case of problems Patchwork should be manually load before tests for this very reason.

Thanks for your contribution. Going to close and then release new version. It will be 1.3 because there are 2 new functions to mock functions: justEcho() and echoArg() (thanks to @tfrommen)

Be sure your requirements allow 1.3 to pull this change.

Thanks again.

FYI I've got patch for Patchwork loading (PR to follow)