Update filters to be reset-able #785

Merged
merged 1 commit into from Jan 14, 2013

Projects

None yet

3 participants

@blainesch
Union of RAD member

This is mainly for testing/mocking purposes. For instance I was Mocking how the static class Session worked. In each test I was setting Session::read to return specific data. This worked great for a single test, however the filters are executed in insert order so the filter in my second test was called last.

@blainesch
Union of RAD member

Seems like an unrelated test failed that cached all the StaticObject methods and complexities...

@jails
Union of RAD member

Yeah it would be interesting to have such feature for tests. But what do you think about having this integrated direclty in applyFilter (see #591 nate's comment) ?

@blainesch
Union of RAD member

I like that implementation. Give me a minute to refactor.

@blainesch
Union of RAD member

Fixed, however I'm not a huge fan of how the complexity test caches methods and complexities. I feel that puts an extra learning overhead when attempting to modify StaticObject. It might be wise to refactor it to do complexity on a FooComplexityClass or something.

@jails jails and 1 other commented on an outdated diff Jan 13, 2013
tests/cases/core/ObjectTest.php
@@ -213,6 +213,53 @@ public function testInstanceFalse() {
$this->expectException('/^Invalid class lookup/');
$object->instance(false);
}
+
+ public function testResetMethodFilter() {
+ $obj = new MockMethodFiltering();
+ $obj->applyFilter(false);
+ $obj->applyFilter('method2', function($self, $params, $chain) {
+ return false;
+ });
+
+ $this->assertFalse($obj->method2());
@jails
jails Jan 13, 2013

Looks like this test will pass without your added filter (indeed array() == false). $this->assertIdentical(false, $obj->method2()) is imo the way to go here.

@blainesch
blainesch Jan 13, 2013

Odd, I never checked before but assertFalse is more of a assertEmpty... let me change that. Good catch.

@jails jails commented on an outdated diff Jan 13, 2013
core/StaticObject.php
@@ -40,17 +40,25 @@ class StaticObject {
* @see lithium\core\StaticObject::_filter()
* @see lithium\util\collection\Filters
* @param mixed $method The name of the method to apply the closure to. Can either be a single
- * method name as a string, or an array of method names.
- * @param closure $filter The closure that is used to filter the method.
+ * method name as a string, or an array of method names. Can also be false to reset
@jails
jails Jan 13, 2013

Maybe delete or remove would be better than reset ?

@jails jails commented on an outdated diff Jan 13, 2013
tests/cases/core/ObjectTest.php
+
+ $this->assertFalse($obj->method2());
+
+ $obj->applyFilter('method2', false);
+
+ $this->assertTrue($obj->method2() !== false);
+ }
+
+ public function testResetMultipleFilters() {
+ $obj = new MockMethodFiltering();
+ $obj->applyFilter(false);
+ $obj->applyFilter(array('method2', 'manual'), function($self, $params, $chain) {
+ return false;
+ });
+
+ $this->assertFalse($obj->method2());
@jails
jails Jan 13, 2013

Same comment here

@jails jails commented on an outdated diff Jan 13, 2013
tests/cases/core/ObjectTest.php
+ $this->assertFalse($obj->manual(array()));
+
+ $obj->applyFilter('method2', false);
+
+ $this->assertTrue($obj->method2() !== false);
+ $this->assertFalse($obj->manual(array()));
+ }
+
+ public function testResetClass() {
+ $obj = new MockMethodFiltering();
+ $obj->applyFilter(false);
+ $obj->applyFilter(array('method2', 'manual'), function($self, $params, $chain) {
+ return false;
+ });
+
+ $this->assertFalse($obj->method2());
@jails
Union of RAD member

Except the assertFalse, your PR is fine for me !

@jails jails commented on an outdated diff Jan 13, 2013
tests/cases/core/StaticObjectTest.php
@@ -162,6 +162,53 @@ public function testInstanceFalse() {
$this->expectException('/^Invalid class lookup/');
MockStaticInstantiator::instance(false);
}
+
+ public function testResetMethodFilter() {
+ $class = 'lithium\tests\mocks\core\MockStaticMethodFiltering';
+ $class::applyFilter(false);
+ $class::applyFilter('method2', function($self, $params, $chain) {
+ return false;
+ });
+
+ $this->assertFalse($class::method2());
@jails
jails Jan 13, 2013

see comments for ObjectTest.

Blaine Schmeisser Update filters to be reset-able
This is mainly for testing/mocking purposes. For instance I was Mocking how the static class `Session` worked. In each test I was setting `Session::read` to return specific data. This worked great for a single test, however the filters are executed in insert order so the filter in my second test was called last.
378cb15
@blainesch
Union of RAD member

Pushed

@jails
Union of RAD member

Cool thanks, this PR is ok for me ! There's only need @nateabele or @gwoo validation.

@nateabele nateabele merged commit 80a13bc into UnionOfRAD:dev Jan 14, 2013

1 check passed

Details default The Travis build passed
@blainesch blainesch deleted the unknown repository branch Jan 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment