[Django] #22478: Wrong module name used in build_test
Django
-------------------------------+------------------------
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------
TestClass loading in django.test.simple.build_test cycle through
discovered modules in application, but iterated modules are not used in
the for cycle.
See
https://github.com/django/django/blob/stable/1.7.x/django/test/simple.py#L183
--
Ticket URL: <https://code.djangoproject.com/ticket/22478>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 1
* needs_docs: => 0
Comment:
Tests still needed
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:1>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* needs_tests: 1 => 0
Comment:
Test attached
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:2>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7-beta-1
Keywords: tests, test suite | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* cc: yakky (added)
* keywords: => tests, test suite
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:3>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7-beta-1
Keywords: tests, test suite | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rtnpro):
* cc: rtnpro (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:4>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rtnpro):
* component: Uncategorized => Testing framework
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:5>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rtnpro):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I am able to reproduce the reported bug. I have also reviewed the patches
and tested it on my local codebase of Django running tag 1.7b1. The
patches seem to be working, however, they are not PEP8 compliant.
{{{
$ pep8 tests/test_suite_override/sample_app/
tests/test_suite_override/sample_app/__init__.py:1:24: W292 no newline at
end of file
tests/test_suite_override/sample_app/models.py:4:1: E302 expected 2 blank
lines, found 1
tests/test_suite_override/sample_app/tests/__init__.py:2:27: W292 no
newline at end of file
}}}
So, I'd suggest yakky to make the change PEP8 compliant and submit an
incremental patch.
I will mark this ticket as accepted. If someone has any point to add,
please feel free to share your opinion.
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:6>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by prestontimmons):
The issue wasn't clear to me based on the report. It boiled down to this:
If you define a test in `tests/__init__.py`, and use the old test runner,
a test label like myapp.TestCase fails to resolve to the test case.
Instead, a ValueError is raised:
{{{
ValueError: Test label 'testapp.MyTest' does not refer to a test
}}}
From my testing, this works in the 1.6.x branch but fails in 1.7.x. I
didn't track down the commit that caused the regression.
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:7>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by prestontimmons):
Replying to [comment:2 yakky]:
> Test attached
yakky, it would help if you submitted this as a single patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:8>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by yakky):
Replying to [comment:8 prestontimmons]:
> Replying to [comment:2 yakky]:
> > Test attached
>
> yakky, it would help if you submitted this as a single patch.
Sure.
Will make the code compliancy modifications and resubmit as single patch
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:9>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by yakky):
Sorry, quite new here: is it possible to remove obsolete attachments?
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:10>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by rtnpro):
Hey yakky,
AFAIK, you can replace existing attachment with the same name, but not
with a different name.
Regards,
rtnpro
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:11>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: tests, test suite | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by yakky):
Thanks.
0001-Fixed-22478.-Use-correct-module-variable-in-build_te.2.patch is the
complete patch (code -linted- and test)
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:12>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | 1.7-beta-1
Keywords: tests, test suite | Resolution:
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
Easy pickings: 0 | Needs documentation: 0
| Patch needs improvement: 1
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by rtnpro):
* stage: Accepted => Ready for checkin
Comment:
https://code.djangoproject.com/attachment/ticket/22478/0001-Fixed-22478
.-Use-correct-module-variable-in-build_te.2.patch looks good to me.
+1.
Marking it as "Ready for checkin".
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:13>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | 1.7-beta-1
Keywords: tests, test suite | Resolution:
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
Easy pickings: 0 | Needs documentation: 0
| Patch needs improvement: 1
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by prestontimmons):
Thanks, yakky. That illustrates the problem well.
I added a pull request here: https://github.com/django/django/pull/2596
I simplified your test case, since test_suite_override isn't really the
right directory for this test, and there is a sample app already with an
`__init__.py`.
Thanks for preparing this.
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:14>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: new
Severity: Normal | 1.7-beta-1
Keywords: tests, test suite | Resolution:
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
Easy pickings: 0 | Needs documentation: 0
| Patch needs improvement: 1
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by yakky):
Good, thanks for support!
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:15>
Django
Reporter: anonymous | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version:
Severity: Normal | 1.7-beta-1
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
Easy pickings: 0 | Needs documentation: 0
| Patch needs improvement: 1
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"935159d9517ca951cbd5665c521d3c91102948d8"]:
{{{
#!CommitTicketReference repository=""
revision="935159d9517ca951cbd5665c521d3c91102948d8"
[1.7.x] Fixed #22478 -- Regression in test label discovery.
As part of the app-loading updates the old test runner was changed to not
require a models module. This introduced a regression in behavior so
applabel.TestCase failed for tests defined in a directory.
The fix is thanks to yakky and rtnpro.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22478#comment:16>