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.

  1. 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).
  2. Tests must verify that deletion works appropriately, particularly recursive deletion.
  3. 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.
  4. 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.
  5. 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.

Comments

marvil07’s picture

Status: Active » Needs work
StatusFileSize
new8.19 KB

I 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:

    Refactor VersioncontrolRepositoryUnitTestingTestCase to test all CRUD.
    
    - Test each table field for repository Creation test.
    - Add tests for Update and Delete.
    - Use directly the controller for Read instead of the backend method.
    - Verify with direct database queries.
    - Assert versioncontrolCreateRepository() method and use it at repo unit
      tests.
marvil07’s picture

StatusFileSize
new12.66 KB

While 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.

sdboyer’s picture

First 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.

marvil07’s picture

The 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.

sdboyer’s picture

OK, sounds like a plan. My problem with your patches is probably related to $Id$ tags, so I need to deal with it.

marvil07’s picture

Title: Create tests that exercise CRUD & methods on all our entities » [meta] Create tests that exercise CRUD & methods on all our entities
Status: Needs work » Active
eliza411’s picture

Issue tags: +git sprint 7

Tagging for Git Sprint 7.

webchick’s picture

Assigned: marvil07 » Unassigned
Issue tags: +git low hanging fruit

This 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.

sdboyer’s picture

Out of phase 2, like all the testing issues.

marvil07’s picture

Priority: Normal » Major
Issue tags: +versioncontrol-6.x-2.0-release-blocker

test ftw on stable

marvil07’s picture

Status: Active » Postponed
Issue tags: -versioncontrol-6.x-2.0-release-blocker

  • marvil07 committed 65ac571 on 7.x-1.x
    Issue #997254: Adds missing test classes to info file.
    
  • marvil07 committed 9117b53 on 7.x-1.x
    Issue #997254: Renames controller caching test file/class now that is...
  • marvil07 committed a399618 on 7.x-1.x
    Issue #997254: Adds initial testing for controller may cache option.
    
  • marvil07 committed f856c6d on 7.x-1.x
    Issue #997254: Fix typo.
    

  • marvil07 committed 4abdbc9 on 7.x-1.x
    Issue #997254: Adds an assert to validate loaded entities from test...
  • marvil07 committed b1fc17e on 7.x-1.x
    Issue #997254: Adds a helper to create a set of versioncontrol object...