Quoting on sdboyer definition at #960414-7: Define which tests to create to exercise CRUD & methods on all our entities:
OK, here's a set of guidelines I think CRUD tests should strive for, in descending order of importance.
- Tests must do super-basic save verification, ensuring that all the data that's supposed to go into the db goes in properly. That especially means the nested insertions of dependent objects. To greatest extent possible, this should be verified to work independent of the loaders (so running direct db queries).
- Tests must verify that deletion works appropriately, particularly recursive deletion.
- Any $options that can be set for any kind of CRUD operation - so loading with the controllers or saving/deleting with the entities themselves - must be tested and verified to work as intended.
- We need to test $conditions on controllers. Not every possible condition, though; only each unique condition-handling logic path. Per controller. So we need just one check for all straight field filters (i.e., $query->condition($alias . $field_name, $value)), and then additional checks for any sort of condition that follows an alternate path.
- Tests should aggressively search for cache inconsistencies/misses, as the immaturity of VersioncontrolEntity::cacheGet generally makes me think we could see a fair number of those, and that's potentially really bad.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | entity-crud-tests-v1.patch | 12.66 KB | marvil07 |
| #1 | 0001-Refactor-VersioncontrolRepositoryUnitTestingTestCase.patch | 8.19 KB | marvil07 |
Comments
Comment #1
marvil07 commentedI committed two minor changes in preparation for this: http://drupal.org/cvs?commit=464394 and http://drupal.org/cvs?commit=464398
Here is a patch that start with 1. for repository entity, which:
Comment #2
marvil07 commentedWhile I was writing an update for this, I end up committing a minor follow-up, and opening two issues: #998676: Add repository update_method UI and #998684: Remove the idea of operation label actions.
What is new in this patch: Basic CRUD tests for tag entity.
Comment #3
sdboyer commentedFirst patch applies clean, tests pass, makes general sense. Second patch doesn't apply, though.
Do we want to do all these tests in a single issue, or split them out into issues for each entity? I don't care either way.
Comment #4
marvil07 commentedThe second patch applies cleanly here. Please notice it is not an incremental patch(on top of the one in #1), it is just a patch on top of last CVS HEAD.
I was wondering also if we want to commit all the tests in one commit, so now that I know your position about it, I will create specific issues when I think this is too big :-p and keep this as meta if needed.
Comment #5
sdboyer commentedOK, sounds like a plan. My problem with your patches is probably related to $Id$ tags, so I need to deal with it.
Comment #6
marvil07 commentedMoving to meta, as this will track the new issues(that IMO cover the initial points):
#1000862: Basic CRUD tests for entities
#1000870: Inter-entities CRUD operations tests for entities
#1000876: Tests for entity controllers
#1000874: Behaviour tests for entities
An updated patch ready for review is on #1000862: Basic CRUD tests for entities.
Comment #7
eliza411 commentedTagging for Git Sprint 7.
Comment #8
webchickThis isn't git low hanging fruit, per se, but it's one of those tasks that people who are NOT sam and marco could conceivably work on, and is super important to the viability of the project.
Unassigning Marco.
Comment #9
sdboyer commentedOut of phase 2, like all the testing issues.
Comment #10
marvil07 commentedtest ftw on stable
Comment #11
marvil07 commented