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

Why expect function works unexpectedly? #88

Open
wppunk opened this issue Oct 22, 2020 · 4 comments
Open

Why expect function works unexpectedly? #88

wppunk opened this issue Oct 22, 2020 · 4 comments
Labels

Comments

@wppunk
Copy link

wppunk commented Oct 22, 2020

When I run this code:

expect( 'func_1' )
	->once()
	->with( 'krya' )
	->andReturn( 'krya' );

$this->assertSame( 'krya', func_1( 'ne-krya' ) );

Result: Failed asserting that null is identical to 'krya'.. It looks well.

But when I run this code:

expect( 'func_1' )
	->with( 'krya' )
	->once()
	->andReturn( 'krya' );

$this->assertSame( 'krya', func_1( 'ne-krya' ) );

OK (1 test, 1 assertion)

Works but why?

@abrain
Copy link

abrain commented Jan 15, 2021

I came across this problem as well and as far as I can tell, this manifests itself like this:

  • Functions\expect does not validate arguments given in with() if there is no times quantifier like once()
  • Adding once() after with() affects andReturn(), but still does not validate arguments given in with()

Code in the SUT (error_log only for demonstration purposes)

$a = get_post_meta($postId, 'a', true);
$b = get_post_meta($postId, 'b', true);
error_log("a is '$a' and b is '$b'");
add_term_meta($termId, 'a', $a, true);
add_term_meta($termId, 'b', $b, true);

once() after with()

This was my initial setup in the test. Note that the get_post_meta calls are in a different order than in the SUT ('b' before 'a').

expect('get_post_meta')->with(1, 'b', true)->once()->andReturn('B');
expect('get_post_meta')->with(1, 'a', true)->once()->andReturn('A');

The error_log in the SUT shows this: a is 'B' and b is 'A'. The values given in andReturn() have been returned in the order they were defined, indepentent of the arguments given in with().

once() before with()

Putting once() before with() did fix this behaviour.

expect('get_post_meta')->once()->with(1, 'b', true)->andReturn('B');
expect('get_post_meta')->once()->with(1, 'a', true)->andReturn('A');

The error_log in the SUT shows: a is 'A' and b is 'B'

Without once()

Leaving out once() causes the arguments in with() to be ignored and the value in andReturn() is returned for every call to this function.

expect('get_post_meta')->with(1, 'b', true)->andReturn('B');
expect('get_post_meta')->with(1, 'a', true)->andReturn('A');

The error_log in the SUT shows: a is 'B' and b is 'B'

The incorrect value (a = B) should be discovered by this test:

expect('add_term_meta')->with(5, 'a', 'A', true);
expect('add_term_meta')->with(5, 'b', 'B', true);

This check will not fail, because once() has been omitted.

Conclusion

In case with() is used, Functions\expect should enforce the use of a times quantifier like once() as the first call after expect(). Alternatively, once() could be the default if no quantifier is passed.

Currently, checking passed arguments with with() while not using once() before with() causes the test to pass, even when the values do not match.

@gmazzap
Copy link
Collaborator

gmazzap commented Dec 12, 2022

@wppunk @abrain, I'm sorry for the late answer here.

Has anyone of you tested if this is something that applies to Mockery or it is something that only happens on Brain Monkey?

@paulshryock
Copy link
Contributor

paulshryock commented Feb 28, 2023

I can confirm that this is an issue, and that @abrain's assessment above is correct.

expect()
  ->once()       // Must be present, and must be first
  ->with()       // Otherwise this is not validated
  ->andReturn(); // When this is present

@paulshryock
Copy link
Contributor

paulshryock commented Feb 28, 2023

@wppunk @abrain, I'm sorry for the late answer here.

Has anyone of you tested if this is something that applies to Mockery or it is something that only happens on Brain Monkey?

I've just validated that this issue does not apply to Mockery.

(Mockery::mock('SomeClass'))
  ->shouldReceive()
  ->with()       // This is validated, without needing `->once()`
  ->andReturn(); // And this is present

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants