merged branch char101/master (PR #844)
authorFabien Potencier <fabien.potencier@gmail.com>
Fri, 28 Sep 2012 21:45:13 +0000 (23:45 +0200)
committerFabien Potencier <fabien.potencier@gmail.com>
Fri, 28 Sep 2012 21:45:13 +0000 (23:45 +0200)
commit4ba7ae939cbd2124d13e97fe75bc061a881aa50f
tree5ccbb393ed3594f2b6287618a06df1054a94929e
parent8d14fa8290d3958ae59436e0cbb5dd59e74413c8
parentc23ef25c8c1e0d1ce939b86b1ce2d89eb1a1c138
merged branch char101/master (PR #844)

This PR was merged into the master branch.

Commits
-------

c23ef25 Add assertEquals to NativeExtensionTest.php
9126dc6 Twig extension: fix case when accessing property of an array casted into object
34cf8e1 Fix double free
4980903 Enhancements for twig extension
db3cb80 Fix NativeExtensionTest
3485ee7 Native extension: handle dynamic properties defined in the get_properties handler in a per instance fashion.

Discussion
----------

Native extension: call get_properties in per instance manner instead of caching it.

Since dynamic properties of an object can be defined by its get_properties handler, we need to call it for each instance.

---------------------------------------------------------------------------

by char101 at 2012-09-21T10:30:37Z

PHPUnit test result

```
PHPUnit 3.7.1 by Sebastian Bergmann.

.S...........................................................   61 / 1253 (  4%)
.............................................................  122 / 1253 (  9%)
.............................................................  183 / 1253 ( 14%)
.............................................................  244 / 1253 ( 19%)
.............................................................  305 / 1253 ( 24%)
.............................................................  366 / 1253 ( 29%)
.............................................................  427 / 1253 ( 34%)
.............................................................  488 / 1253 ( 38%)
.............................................................  549 / 1253 ( 43%)
.............................................................  610 / 1253 ( 48%)
.............................................................  671 / 1253 ( 53%)
.............................................................  732 / 1253 ( 58%)
.............................................................  793 / 1253 ( 63%)
.............................................................  854 / 1253 ( 68%)
.............................................................  915 / 1253 ( 73%)
.............................................................  976 / 1253 ( 77%)
............................................................. 1037 / 1253 ( 82%)
............................................................. 1098 / 1253 ( 87%)
............................................................. 1159 / 1253 ( 92%)
............................................................. 1220 / 1253 ( 97%)
.................................

Time: 4 seconds, Memory: 13.25Mb

OK, but incomplete or skipped tests!
Tests: 1253, Assertions: 2969, Skipped: 1.
```

---------------------------------------------------------------------------

by stof at 2012-09-21T11:58:51Z

@char101 My previous comment about the way the test should be implemented is still valid. Please rewrite it to use the same way to all other integration tests in Twig

---------------------------------------------------------------------------

by char101 at 2012-09-23T04:51:36Z

@stof I don't see the reason of using a fixture. The test case works, it accomplishes its goal. It's simple. It doesn't test for a feature, it tests for a specific case where PHP crashes.

---------------------------------------------------------------------------

by stof at 2012-09-23T13:53:41Z

@char101 I see one: you are building a Twig instance and rendering a template here, which is exactely what the integration tests are doing.
Btw, your test would fail when running phpunit in strict mode as it does not assert anything

---------------------------------------------------------------------------

by char101 at 2012-09-24T02:05:58Z

@stof I don't have the desire to change what isn't broken, but you are free to change it as you see fit. As for the assert, I have added it to the test.