-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Review and merge Mapping Drivers tests #2348
Comments
Placing classes in same file is great solution to be sure they're not reused across tests. Personally I like this approach but am open to changing it given sensible solution :)
This test probably remembers time when there also was a YAML metadata driver and its task was to ensure that every mapping option works for both drivers.
I will trust your best judgement here as you recently dived into mapping stuff deep enough to add attribute driver :) |
You are right but in this case, all the documents should be shared between all driver tests so we can use same assertions on same documents, but with different drivers. Maybe some of them could be placed in the test file itself, but I'd reserve it only for some edge cases or testing driver-specific features (like different ways to map Indexes in AnnotationDriver).
Yes, it seems that there used to be the YamlDriver test there that compared the results with XmlDriver but since it's gone now, it doesn't really add much value. I will try to come up with some solution for this. Should I target 2.3 (and higher)? |
Please do, let's have attribute driver benefit from this already :) |
Improvement Request
Summary
Currently there are 2 separate namespaces containing Mapping Drivers tests:
Doctrine\ODM\MongoDB\Tests\Mapping
andDoctrine\ODM\MongoDB\Tests\Mapping\Driver
:The first one derive from https://github.com/doctrine/mongodb-odm/blob/2.2.x/tests/Doctrine/ODM/MongoDB/Tests/Mapping/AbstractMappingDriverTest.php class, tests both AnnotationDriver and XmlDriver, seems newer, but all tested Document classes are placed in the abstract test class file.
The second one is just one test class that extends https://github.com/doctrine/mongodb-odm/blob/2.2.x/tests/Doctrine/ODM/MongoDB/Tests/Mapping/Driver/AbstractDriverTest.php, does just some simple checks, tests only the XML driver, is very old (remembers 2010), but the fixture files are placed in a separate directory: https://github.com/doctrine/mongodb-odm/tree/2.2.x/tests/Doctrine/ODM/MongoDB/Tests/Mapping/Driver/fixtures which makes it much cleaner and easier to refactor.
I believe these tests should be merged together, probably kept only the new classes but fixtures should be extracted into separate files for easier maintainability.
The text was updated successfully, but these errors were encountered: