[WIP] added a test for identifier getter in a trait #337
Conversation
…n for lazy loading
|
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DCOM-251 We use Jira to track the state of pull requests and the versions they got |
| * @since 2.4 | ||
| */ | ||
| class LazyLoadableObject | ||
| { | ||
| use IdentifierTrait; |
There was a problem hiding this comment.
you should use a separate fixture class for the test involving trait, so that the existing test can still run on 5.3 and only the test related to traits need to be skipped
…their own classes and could be skipped if php < 5.4.0
|
As discussed on IRC, these tests are not sustainable as they are written in this PR. |
|
@Ocramius what are the only methods you think should exist in the traits test ? |
|
@dorongutman I'm just disagreeing with duplicating the entire test |
| 'publicIdentifierField' => true, | ||
| 'protectedIdentifierField' => true, | ||
| 'publicPersistentField' => true, | ||
| 'protectedPersistentField' => true, |
There was a problem hiding this comment.
@stof this is not yet ready for review
Ocramius
changed the title
|
you should indeed not copy/paste the whole test. Rely on PHPUnit data providers where needed. thus, your issue only concern 1 test (i.e. the one failing currently on Travis) so only this test should be added. All other duplicated tests are useless |
|
Closing. A solution to this (without going completely crazy) would be doctrine/orm#1241, but we're far from implementing it. |
When the identifier getter method is in a trait, the proxy generation ignores it and treats it as a lazy loading field, causing the $proxyInstance->getId() to initialize the object.
This is a test that proves that.