Skip to content

Commit

Permalink
Add in escape valves for backwards compatability
Browse files Browse the repository at this point in the history
This adds in two specific escape mechanisms to enable backwards
compatability in two areas where Shmock has become more strict.

The first escape valve allows users to turn off 'strict method
checking' when mocking static methods, which previously was allowed
before Shmock 2.0.0-alpha6.

The seconds escape valve allows users to turn off Shmock policy
validation on mocked instance methods where no parameters are passed
in the shmock closure. This has always been broken until now.

This isn't the most pleasant, but it is needed for full adoption
in upstream codebases.
  • Loading branch information
jmarrama committed Sep 24, 2015
1 parent 6c115af commit 4202dac
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 13 deletions.
9 changes: 0 additions & 9 deletions src/Shmock/ClassBuilderInstanceClass.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ class ClassBuilderInstanceClass extends ClassBuilderStaticClass
*/
protected $disable_original_constructor = false;

/**
* @var callable
* @internal
* If we want to shmock the static context of a shmock'd object
* we need to call get_class() on the final mock, so we save
* any configuration closure until after everything is done.
*/
protected $shmock_class_closure = null;

/**
* Arguments that will be passed to the original constructor.
* @var array
Expand Down
13 changes: 13 additions & 0 deletions src/Shmock/InstanceSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,17 @@ protected function isStatic()
{
return false;
}

/**
* This is pretty nasty - this is only included for backwards compatibility reasons. Skip
* checking function args against policies if no args are passed and the relevant flag has
* been set in Shmock. See the comment on the flag in the Shmock class for more context
*
* @return bool
*/
protected function shouldCheckArgsAgainstPolicies()
{
return count($this->arguments) > 0 || Shmock::$check_args_for_policy_on_instance_method_when_no_args_passed;
}

}
15 changes: 15 additions & 0 deletions src/Shmock/Shmock.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ class Shmock
*/
private static $outstanding_shmocks = [];

/**
* @deprecated - this is only included for backwards compatability reasons. Before Shmock v2, shmock
* policies on instance method arguments used to not be checked if no method arguments were passed.
* Please only change this sparingly!
* @var bool
*/
public static $check_args_for_policy_on_instance_method_when_no_args_passed = true;

/**
* @deprecated - this is only included for backwards compatability reasons. Before Shmock 2.0.0-alpha6,
* 'strict' method checking did not happen on mocked static methods. Please only change this sparingly!
* @var bool
*/
public static $disable_strict_method_checks_for_static_methods = false;

/**
* Create an instance of a mock object. Shmock uses a build / replay model for building mock objects.
* The third argument to the create method is a callable that acts as the mock's build phase. The resulting
Expand Down
30 changes: 26 additions & 4 deletions src/Shmock/StaticSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class StaticSpec implements Spec
/**
* @var array arguments
*/
private $arguments;
protected $arguments;

/**
* @var callable
Expand Down Expand Up @@ -71,13 +71,35 @@ public function __construct($testCase, $className, $methodName, $arguments, $pol
$this->frequency = new CountOfTimes(1, $this->methodName);
$this->policies = $policies;

$this->doStrictMethodCheck();
if ($this->shouldDoStrictMethodCheck()) {
$this->doStrictMethodCheck();
}

foreach ($this->policies as $policy) {
$policy->check_method_parameters($className, $methodName, $arguments, $this->isStatic());
if ($this->shouldCheckArgsAgainstPolicies()) {
foreach ($this->policies as $policy) {
$policy->check_method_parameters($className, $methodName, $arguments, $this->isStatic());
}
}
}

/**
* This is a backwards compatability escape valve to prevent strict method checks when
* a user requests it for static methods. See the comment in the Shmock class for more context
* @return bool
*/
private function shouldDoStrictMethodCheck()
{
return !($this->isStatic() && Shmock::$disable_strict_method_checks_for_static_methods);
}

/**
* @return bool
*/
protected function shouldCheckArgsAgainstPolicies()
{
return true;
}

/**
* @return bool
*/
Expand Down
23 changes: 23 additions & 0 deletions test/Shmock/Policy_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,25 @@ public function test_policy_allows_valid_throws()
$calculator->raise_to_even(5);
}

/**
* @expectedException \InvalidArgumentException
*/
public function test_arg_policy_throws_when_no_args_passed()
{
$this->shmock('Shmock\Even_Calculator', function ($calculator) {
$calculator->raise_to_even();
});
}

public function test_arg_policy_does_not_throw_when_no_args_passed_and_escape_flag_set()
{
Shmock::$check_args_for_policy_on_instance_method_when_no_args_passed = false;
$calc = $this->shmock('Shmock\Even_Calculator', function ($calculator) {
$calculator->raise_to_even()->return_value(6);
});
Shmock::$check_args_for_policy_on_instance_method_when_no_args_passed = true;
$this->assertEquals($calc->raise_to_even(13), 6);
}
}

class Even_Calculator
Expand Down Expand Up @@ -107,6 +126,10 @@ class Even_Number_Policy extends Policy
{
public function check_method_parameters($class, $method, $parameters, $static)
{
if (count($parameters) == 0) {
throw new \InvalidArgumentException("No args passed!");
}

if (!is_integer($parameters[0])) {
throw new Shmock_Exception();
}
Expand Down
47 changes: 47 additions & 0 deletions test/Shmock/ShmockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,43 @@ public function testShmockCanHandleOrderMattersAndThrowExceptionWhenWrong()
$this->assertTrue($thrown);
}

public function testShmockCannotMockStaticPrivateMethodsNormally()
{
$testCaseShmock = Shmock::create(
$this,
'\PHPUnit_Framework_TestCase',
function ($mock) {
$mock->shmock_class(function ($smock) {
$smock->assertFalse(true); // this would normally cause the test to fail, this ensures that it normally would happen
});
}
);

$foo = Shmock::create_class(
$testCaseShmock,
'\Shmock\Shmock_Foo',
function ($mock) {
$mock->sBar()->return_value(2);
}
);

$foo::sFoo();
}

public function testShmockCanMockStaticPrivateMethodsWithEscapeFlag()
{
Shmock::$disable_strict_method_checks_for_static_methods = true;
$foo = Shmock::create_class(
$this,
'\Shmock\Shmock_Foo',
function ($mock) {
$mock->sBar()->return_value(2);
}
);
Shmock::$disable_strict_method_checks_for_static_methods = false;

$this->assertEquals($foo::sFoo(), 2);
}
public function tearDown()
{
Shmock::verify();
Expand All @@ -193,6 +230,16 @@ public function __construct()
$this->foo = "42";
}

public static function sFoo()
{
return static::sBar();
}

private static function sBar()
{
return 42;
}

public function lala()
{
return static::weewee();
Expand Down

0 comments on commit 4202dac

Please sign in to comment.