Conversation
|
Yeah these failures are expected. Going to try see if the fix can be made in django itself. Just made PR now, to include failing test. e: Here's the current status on the Django ticket: https://code.djangoproject.com/ticket/30427#ticket |
|
can you add a isort rule to the makefile too? |
| value = self.descriptor.__get__(instance, owner) | ||
| except AttributeError: | ||
| value = self.descriptor | ||
| value = self.descriptor.__get__(instance, owner) |
There was a problem hiding this comment.
| value = self.descriptor.__get__(instance, owner) | |
| try: | |
| value = self.descriptor.__get__(instance, owner) | |
| except AttributeError: | |
| value = self.descriptor |
There was a problem hiding this comment.
Hey, this just reverts my changes?
I've already forgotten a lot of what was going on here, but I believe the try-catch was only a bandaid fix that caused other edge cases to fail.
I've not used makefiles this way before, but can take a look when it's time to finally look into getting this merged in. I think we're blocked on django currently. See my open ticket and PR with them here: https://code.djangoproject.com/ticket/30427#ticket |
|
Closing. Idk if this was addressed in the end by someone else somewhere else, but I don't have the resources to delve into here again.. ;/ |
There used to be a [bug in Django](https://code.djangoproject.com/ticket/30427) that caused these tests to fail, according to the discussion of jazzband#370. That bug was fixed in Django 4.0. However, because the test model was using a `FieldTracker` rather than a `ModelTracker`, all model-specific test cases in `ModelTrackerTests` continued to fail.
This catch was introduced in jazzband#367, but subsequent discussion in jazzband#370, jazzband#381 and jazzband#382 suggests that it wasn't the proper solution. Now that `AbstractModelTrackerTests` is re-enabled and passing, it becomes clear that `AttributeError` is no longer raised during unit testing when using a recent Django version. Therefore I think it is safe to remove the `try`/`except`.
There used to be a [bug in Django](https://code.djangoproject.com/ticket/30427) that caused these tests to fail, according to the discussion of jazzband#370. That bug was fixed in Django 4.0. However, because the test model was using a `FieldTracker` rather than a `ModelTracker`, all model-specific test cases in `ModelTrackerTests` continued to fail.
This catch was introduced in jazzband#367, but subsequent discussion in jazzband#370, jazzband#381 and jazzband#382 suggests that it wasn't the proper solution. Now that `AbstractModelTrackerTests` is re-enabled and passing, it becomes clear that `AttributeError` is no longer raised during unit testing when using a recent Django version. Therefore I think it is safe to remove the `try`/`except`.
Reverts #367
Adds a test for #331
PR also sneaks in some linter configs.
Issue not fixed here. I suspect it's a django issue. Findings reported in #331.
CHANGES.rstfile to describe the changes, and quote according issue withGH-<issue_number>.