Wrong module name used in build_test

[anonymous]

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

[Iacopo Spalletti]

Code fix

[Iacopo Spalletti]

Tests still needed

[Iacopo Spalletti]

Test

[Iacopo Spalletti]

Test attached

[rtnpro]

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.

[Preston Timmons]

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.

[Preston Timmons]

Replying to yakky:

Test attached

yakky, it would help if you submitted this as a single patch.

[Iacopo Spalletti]

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

[Iacopo Spalletti]

Code fix + Test

[Iacopo Spalletti]

Sorry, quite new here: is it possible to remove obsolete attachments?

[rtnpro]

Hey yakky,

AFAIK, you can replace existing attachment with the same name, but not with a different name.

Regards,
rtnpro

[Iacopo Spalletti]

Thanks.
0001-Fixed-22478.-Use-correct-module-variable-in-build_te.2.patch is the complete patch (code -linted- and test)

[rtnpro]

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".

[Preston Timmons]

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.

[Iacopo Spalletti]

Good, thanks for support!

[Tim Graham <timograham@…>]

In 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.