DDC-2644: "final" declaration of ORM\Query class prevents proper unit testing of repositories/models #3379

Closed
doctrinebot opened this Issue Aug 31, 2013 · 3 comments

5 participants

@doctrinebot

Jira issue originally created by user arron:

For proper unit testing it is neccesary to mock all the dependencies from tested class. In repositories/models, one of the dependencies is ORM\Query class. Because it is declared as final, it is neccesary (in order to perform at least some unit test) to create fake entity manager and than lot of assertions. This all won't be needed if ORM\Query won't be declared final, mock can be created and no additional work won't be neccessary in order to create better unit test.

@beberlei beberlei was assigned by doctrinebot Dec 6, 2015
@betaglop

This issue (that the ORM\Query class is declared as final) also make us wondering:
We need to implement the ILIKE operator (for PostgreSQL) in Lexer, and then we would need to extend it, so we would need also to extend the Parser... And to do this, we would need to use a Query that inherits from the ORM\Query class. Btw, it is not possible.

I'm not sure we need to do what I've just explained, but this should happen in other situations... And it surprises me.

@Ocramius
Doctrine member

We need to implement the ILIKE operator (for PostgreSQL) in Lexer, and then we would need to extend it, so we would need also to extend the Parser...

ILIKE can be implemented as a DQL function

And to do this, we would need to use a Query that inherits from the ORM\Query class.

Query is not designed to be extended: making it final is simply making this decision explicit.

As for the original issue

For proper unit testing

DQL queries are not mockable: repository methods are mockable.
SQL/DQL are non-abstracted code that cannot be worked around: executing the code is the proper way to test those. This indeed leads to slow tests that cause interactions with a database, but that's the correct way of doing it. If you need to stub something, stub the repository method that abstracts the access to the persistence layer instead.

Closing here as won't fix.

@Ocramius Ocramius closed this Jan 6, 2016
@Ocramius Ocramius added the Won't Fix label Jan 6, 2016
@Ocramius Ocramius assigned Ocramius and unassigned beberlei Jan 6, 2016
@Ocramius Ocramius added the Invalid label Jan 6, 2016
@deeky666
Doctrine member

@betaglop the Query and Parser classes are not meant to be extended in userland. Therefore Query class is marked as final. However there are possibilities to extend the DQL with custom DQL functions. You could implement a custom DQL function that generates your desired ILIKE expression I suppose. Looks like there already is an example at stackoverflow.

Concerning the original issue: Opening classes for extension just for the sake of testability is not a good practice and we won't do that. Mocking ORM query component is generally not advisable. I guess in most cases where you need this is when testing repositories and those should rather be tested integrational in userland anyways. In your own application when using a third party library like Doctrine you don't want to mock the already well tested third party components but write integration tests that test whether your application code integrates third party code correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment