Opened 11 years ago
Closed 11 years ago
#22478 closed Bug (fixed)
Wrong module name used in build_test
Reported by: | anonymous | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | 1.7-beta-1 |
Severity: | Normal | Keywords: | tests, test suite |
Cc: | Iacopo Spalletti, rtnpro | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
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
Attachments (3)
Change History (19)
by , 11 years ago
Attachment: | 0001-Fixed-22478.-Use-correct-module-variable-in-build_te.patch added |
---|
comment:3 by , 11 years ago
Cc: | added |
---|---|
Keywords: | tests test suite added |
comment:4 by , 11 years ago
Cc: | added |
---|
comment:5 by , 11 years ago
Component: | Uncategorized → Testing framework |
---|
comment:6 by , 11 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
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.
comment:7 by , 11 years ago
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.
follow-up: 9 comment:8 by , 11 years ago
comment:9 by , 11 years ago
Replying to prestontimmons:
Replying to 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
by , 11 years ago
Attachment: | 0001-Fixed-22478.-Use-correct-module-variable-in-build_te.2.patch added |
---|
Code fix + Test
comment:11 by , 11 years ago
Hey yakky,
AFAIK, you can replace existing attachment with the same name, but not with a different name.
Regards,
rtnpro
comment:12 by , 11 years ago
Thanks.
0001-Fixed-22478.-Use-correct-module-variable-in-build_te.2.patch is the complete patch (code -linted- and test)
comment:13 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
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".
comment:14 by , 11 years ago
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.
comment:16 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Code fix