meshy / django-conman

NOT READY: Work in progress. A content management system for django
BSD 2-Clause "Simplified" License
3 stars 5 forks source link

Show and remove deprecation warnings in tests #47

Closed meshy closed 8 years ago

meshy commented 9 years ago

Blocked by #44

kevinetienne commented 9 years ago

It looks like there're 3 other warnings (2 for django 1.9 and 1 for django 1.10):

/home/travis/build/meshy/django-conman/conman/redirects/handlers.py:7: 
RemovedInDjango19Warning: Default value of 'RedirectView.permanent' 
will change from True to False in Django 1.9. Set an explicit value 
to silence this warning.

  view = views.RouteRedirectView.as_view()
/home/travis/build/meshy/django-conman/conman/redirects/tests/test_handlers.py:17: 
RemovedInDjango19Warning: Default value of 'RedirectView.permanent' 
will change from True to False in Django 1.9. Set an explicit value 
to silence this warning.

  expected = RouteRedirectView.as_view()
/home/travis/build/meshy/django-conman/conman/routes/tests/urls.py:12: 
RemovedInDjango110Warning: Support for string view arguments to url() is deprecated
and will be removed in Django 1.10 (got conman.routes.tests.urls.dummy_view). Pass
the callable instead.

  url(r'^$', view_path),
meshy commented 9 years ago

Yeah, I wasn't quite sure how to deal with those.

The first two relate to the fact that RouteRedirectView does not explicitly set permanent. As it's set dynamically, I'm not sure what to do.

I could add permanent = None, I suppose, but that seems wrong somehow, as it will be overridden.

I could make it a property, and move the logic out of the get_redirect_url method (as this also seems a little wrong to me).

Alternatively, I could just use a function-based view ;) -- It would be even simpler than it currently is, but wouldn't be extensible. Perhaps the question is: how extensible does this need to be?

meshy commented 9 years ago

The third warning is a bit harder -- the whole file only exists as a test, but if I replace the path with the callable function, then I need to find a new way to override it.

meshy commented 9 years ago

Perhaps we just need to run clear_url_caches in the setUp...

LilyFoote commented 9 years ago

I think I would set permanent=False on the RouteRedirectView. This will silence the warning by opting in to the default future behaviour. It doesn't really matter that we always explicitly override it later. This might need a comment though.

meshy commented 9 years ago

I think I would set permanent=False to the RouteRedirectView.

Seems quite reasonable.

kevinetienne commented 9 years ago

Oh I see! Except using a function (or a lambda) which would return a string, I'm not sure what could be used

meshy commented 8 years ago

@KevinEtienne @Ian-Foote I think it might be best to deal with the other warnings in another PR -- you happy with what's here?

LilyFoote commented 8 years ago

Nothing looks wrong on a quick scan.

kevinetienne commented 8 years ago

Looks good to me

meshy commented 8 years ago

Ta!