Skip to content

Add tests for field tracker methods in save method#147

Closed
treyhunner wants to merge 1 commit intomasterfrom
add-missing-tracker-test
Closed

Add tests for field tracker methods in save method#147
treyhunner wants to merge 1 commit intomasterfrom
add-missing-tracker-test

Conversation

@treyhunner
Copy link
Copy Markdown
Member

These tests verify that FieldTracker methods work as expected inside the save() method of a model.

I used a Django signal to inject assertions into a model's save() method and I used a context manager to handle the repetitive signal connecting/disconnecting. Let me know if you think this approach is overkill.

When I cherry-pick 3496fe4 (from #130) onto these changes and run the tests, three of the four tests fail (the .current() function works properly still).

@carljm
Copy link
Copy Markdown
Collaborator

carljm commented Aug 4, 2014

The approach is a bit cleverer than I probably would have implemented, but I think it's reasonable.

Of the two cases broken by #130, though, I think this tests only the less-important one, using FieldTracker after calling super().save() in an overridden save method. This is not guaranteed in the documentation, so if absolutely necessary I think we could consider breaking it.

What is guaranteed in the documentation, but was also broken by #130, and is also untested, is using FieldTracker in a post_save signal handler. Since this is documented to work, I think it's the more important case to add tests for.

@Natim
Copy link
Copy Markdown

Natim commented Aug 21, 2019

I am closing this while we are reworking the FieldTracker feature.

@Natim Natim closed this Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants