Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility fixes for Django 1.10 compatibility, stronger tests, exclude_modeladmins option #8

Merged
merged 18 commits into from
Nov 3, 2016

Conversation

PetrDlouhy
Copy link
Contributor

This PR include all three previously closed PRs. It includes following improvements:
-compatibility fixes with Django 1.10
-stronger tests
-higher test coverage and higher range of versions tested in Travis
-exclude_modeladmins option
-more detailed exception message

since Django<1.8 support was dropped, so this is not tested anymore
it breaks tests when queryset method is defined on modeladmin for compatibility reasons
render respose
add change_view test
@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+4.006%) to 93.75% when pulling 681a261 on PetrDlouhy:master into 53ecdfa on greyside:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+5.4%) to 95.139% when pulling bf8bebd on PetrDlouhy:master into 53ecdfa on greyside:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+5.4%) to 95.139% when pulling 224edab on PetrDlouhy:master into 53ecdfa on greyside:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+7.4%) to 97.101% when pulling 339d234 on PetrDlouhy:master into 53ecdfa on greyside:master.

@PetrDlouhy PetrDlouhy force-pushed the master branch 2 times, most recently from dc6ebda to 3d250fc Compare August 11, 2016 21:23
@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+8.9%) to 98.611% when pulling 3d250fc on PetrDlouhy:master into 53ecdfa on greyside:master.

@PetrDlouhy
Copy link
Contributor Author

@SeanHayes Now I am satisfied with the PR. You can pull :-)

The only line of code, that remains uncovered is exception handling from admin.autodiscover() on line 71. I can't figure out case, when this is used.

@coveralls
Copy link

coveralls commented Aug 17, 2016

Coverage Status

Coverage increased (+8.9%) to 98.611% when pulling 732c655 on PetrDlouhy:master into 53ecdfa on greyside:master.

@coveralls
Copy link

coveralls commented Aug 18, 2016

Coverage Status

Coverage increased (+8.9%) to 98.611% when pulling 86fdb96 on PetrDlouhy:master into 53ecdfa on greyside:master.

@SeanHayes
Copy link
Member

Since this is still being worked on I'll post some minor change requests.

from django.test import TestCase
from django.test.client import RequestFactory
import sys
import django
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this above the other django imports?

@@ -194,12 +220,28 @@ def test_add_view(self, model, model_admin):
# make sure no errors happen here
try:
response = model_admin.add_view(request)
if response.__class__ == django.template.response.TemplateResponse:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of response.__class__ ==, could you use isinstance()?

@PetrDlouhy
Copy link
Contributor Author

@SeanHayes Fixed. I have added flake8-import-order plugin to Tox checks to ensure import order in future editions.

@coveralls
Copy link

coveralls commented Aug 18, 2016

Coverage Status

Coverage increased (+8.9%) to 98.611% when pulling 9b92f9e on PetrDlouhy:master into 53ecdfa on greyside:master.

@PetrDlouhy
Copy link
Contributor Author

@SeanHayes: Any update about this PR? All your issues are fixed.

@coveralls
Copy link

coveralls commented Oct 3, 2016

Coverage Status

Coverage increased (+7.8%) to 97.531% when pulling ac31eff on PetrDlouhy:master into 53ecdfa on greyside:master.

@PetrDlouhy PetrDlouhy force-pushed the master branch 2 times, most recently from 3d250fc to 9cc8ee2 Compare October 4, 2016 14:28
@coveralls
Copy link

coveralls commented Oct 4, 2016

Coverage Status

Coverage increased (+7.8%) to 97.531% when pulling 9cc8ee2 on PetrDlouhy:master into 53ecdfa on greyside:master.

@coveralls
Copy link

coveralls commented Oct 4, 2016

Coverage Status

Coverage increased (+7.8%) to 97.531% when pulling 08f345a on PetrDlouhy:master into 53ecdfa on greyside:master.

@SeanHayes SeanHayes merged commit 746382c into greyside:master Nov 3, 2016
PetrDlouhy referenced this pull request in PetrDlouhy/django-admin-smoke-tests Aug 15, 2022
…clude_modeladmins option (#8)

* remove old queryset testing

since Django<1.8 support was dropped, so this is not tested anymore
it breaks tests when queryset method is defined on modeladmin for compatibility reasons

* stronger tests

render respose
add change_view test

* add exclude_modeladmins option

* test also search request to identify bad search_fields settings

* test in Django 1.10; show all warnings

* fix tests in Django 1.10

* test that exceptions are raised, throw own exception class, increase coverage

* include python 3.4, django 1.7 Travis test

* remove old deprecated code

* test and fix catching PermissionDenied exception - use it where it is needed

* fix backward compatibility with Python 2.7

* print python version to ensure correct testing

* test request.GET parameter

* check import order with flake8; fix all import orders

* use isinstance instead of __class__

* run tests with USE_TZ=True (add pytz dependency to tox)

* fix tox Django version order

* test also some aspects of posting change_view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants