[Django] #24562: Test failure: Pickleability
Django
-------------------------------------------+------------------------
Reporter: shaib | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------------+------------------------
Following the version bump to 1.8.1, we have failures on
`test_unsupported_unpickle (queryset_pickle.tests.PickleabilityTestCase)`
and `test_unsupported_unpickle
(model_regress.test_pickle.ModelPickleTestCase)`. The problem is arguably
in the test code: Both tests assume that
`django.utils.version.get_major_version()` returns a `x.y` string, but
that is only true for .0 minor versions -- and so the tests broke on the
version bump to 1.8.1.
I'm not quite sure about the fix --
`django.utils.version.get_major_version()` looks like a public interface
but it is not documented as far as I could see; it hasn't changed in a
long time, but its return value is unintuitive, and I would expect the
same as the author(s) of these tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24562>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------+------------------------------------
Reporter: shaib | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by bmispelon):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:1>
Django
---------------------------------+-------------------------------------
Reporter: shaib | Owner: timgraham
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* owner: nobody => timgraham
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/4432 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:2>
Django
---------------------------+-------------------------------------
Reporter: shaib | Owner: timgraham
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by shaib):
* severity: Release blocker => Normal
Old description:
> Following the version bump to 1.8.1, we have failures on
> `test_unsupported_unpickle (queryset_pickle.tests.PickleabilityTestCase)`
> and `test_unsupported_unpickle
> (model_regress.test_pickle.ModelPickleTestCase)`. The problem is arguably
> in the test code: Both tests assume that
> `django.utils.version.get_major_version()` returns a `x.y` string, but
> that is only true for .0 minor versions -- and so the tests broke on the
> version bump to 1.8.1.
>
> I'm not quite sure about the fix --
> `django.utils.version.get_major_version()` looks like a public interface
> but it is not documented as far as I could see; it hasn't changed in a
> long time, but its return value is unintuitive, and I would expect the
> same as the author(s) of these tests.
New description:
`django.utils.version.get_major_version()` should returns a `x.y` string,
that is what we call a major version. It does that only for the ".0"
versions -- It returns "1.8" for 1.8, and "1.8.1" for 1.8.1.
I'm not quite sure about the fix --
`django.utils.version.get_major_version()` looks like a public interface
but it is not documented. Now that the tests have been fixed, its only use
in the Django code base is in the same file (from `get_version()`).
Should we just make it private?
--
Comment:
I see that the PR is already merged, so there are no longer test failures
and what's left is not a release blocker.
I updated the summary and description to focus on the issues left.
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:3>
Django
---------------------------+-------------------------------------
Reporter: shaib | Owner: timgraham
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* has_patch: 1 => 0
Comment:
Sorry, you created the ticket after I submitted the PR and I forgot to
update the commit message. Anyway, yes, I think we could just revert the
addition of the `get_major_version()` method as it was added in the same
commit as the tests that used it
(42736ac8e8c31137131714013951249a09e6e7d4).
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:4>
Django
Reporter: shaib | Owner: timgraham
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
* type: Bug => Cleanup/optimization
Comment:
Well, it might be useful to leave the method but rename it to reflect its
behavior: [https://github.com/django/django/pull/4437 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: shaib | Owner: timgraham
Cleanup/optimization |
Component: Utilities | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ad53213066ad25545959170f40322fa5b094989b" ad532130]:
{{{
#!CommitTicketReference repository=""
revision="ad53213066ad25545959170f40322fa5b094989b"
Fixed #24562 -- Renamed get_major_version() to get_main_version()
This reflects the actual behavior of the method.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24562#comment:6>