uber / charlatan

A Python library to efficiently manage and install database fixtures
https://charlatan.readthedocs.org/en/latest/
Other
89 stars 12 forks source link

Uninstall fixture #15

Closed jordanb closed 10 years ago

jordanb commented 10 years ago

This feature adds the capability to uninstall fixtures using one or a set of fixture keys.

We know that the FixturesManager class has a save_instance method and several install_fixture related methods. In a similar way and for parity sake we add a delete_instance as well as several uninstall_fixture methods.

Why is that useful?

This is how things were before:

class TestToaster(unittest.TestCase, charlatan.FixturesManagerMixin):

    def setUp(self):
        self.install_fixture('toast') # this feels magical

    def tearDown(self):
        toast_fixture = self.get_fixture('toast')
        toast = Toast.get(Toast.id == toast_fixture.id)
        self.session.delete(toast)
        self.session.commit() # why not the same magical pattern?

And now you'll be able to do:

class TestToaster(unittest.TestCase, charlatan.FixturesManagerMixin):

    def setUp(self):
        self.install_fixture('toast') # this feels magical

    def tearDown(self):
        self.uninstall_fixture('toast') # this also feels magical

How the magic works?

When calling FixturesManager.delete_instance, the logic expects (and hopes) that your model instance will have a Model.delete_instance() method or a Model.delete() method. If it has both, only Model.delete_instance will be called. This works for peewee and other standard orm. It is very similar to how the FixturesManager.save_instance method relies on the Model.save() method being defined by your orm.

Limitations

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 4b28afaad5d858f4c7b5da72ded6ef6add52bc09 on uninstall_fixture into 145707fbb4bdc3e7e0a90cf188f49b954bc9a4a3 on master.

charlax commented 10 years ago

I'm still uncomfortable with this, as it is too framework specific... What do you think of using this: http://peewee.readthedocs.org/en/latest/peewee/playhouse.html#test-utils

charlax commented 10 years ago

Will still think through this though.

jordanb commented 10 years ago

Thank you for your feedback. This feature was not meant to support peewee only. It does have assumptions about the framework but:

  1. it is not more or less framework specific than relying on the existence of a save method on the models, like charlatan does currently
  2. most ORMs rely on a delete_instance or delete method
  3. anyway, it provides hooks in order to customize the needs for ORMs that are not relying on these methods to delete instances (a before_delete hook was indeed added)
  4. charlatan is already very tightly coupled to SQLAlchemy, maybe some refactoring with adapters is necessary in that case to be totally ORM agnostic and not rely on the existence of any methods

@charlax Let me know what you think.

charlax commented 10 years ago

Looking great, left a few comments. Making it totally ORM agnostic would be great but probably out of scope.

Can you add some more tests and a bit more documentation?

Thanks a lot Jordan!

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling e23aa4ed0ffaa1681ca070a554bed9079938ab13 on uninstall_fixture into 145707fbb4bdc3e7e0a90cf188f49b954bc9a4a3 on master.