Skip to content

Conversation

@tjaeni01
Copy link

@tjaeni01 tjaeni01 commented Oct 27, 2022

There are still warnings present
RemovedInDjango41Warning: 'django_extensions' defines default_app_config = 'django_extensions.apps.DjangoExtensionsConfig'. Django now detects this configuration automatically. You can remove default_app_config.
It seems to be that two packages that we use (django_extensions and webpack_loader) define default_app_config, which is not needed anymore. We might have to wait for a new version for them to remove it

Resolves #29

@tjaeni01 tjaeni01 marked this pull request as ready for review October 27, 2022 14:59
@tjaeni01 tjaeni01 requested a review from alvinsiu October 27, 2022 14:59

urlpatterns += [
url(r'^__debug__/', include(debug_toolbar.urls)),
re_path(r'^__debug__/', include(debug_toolbar.urls)),
Copy link
Member

Choose a reason for hiding this comment

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

I think path might be more appropriate (and remove the regex components): https://django-debug-toolbar.readthedocs.io/en/latest/installation.html#add-the-urls

Comment on lines +6 to 7
pytest
{%- if cookiecutter.feature_annotations == "on" %}
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need pytest-django

Comment on lines +7 to +18
class TestClass:

@pytest.mark.parametrize(
"url_name, url, view_class",
[
("index", "/", views.IndexView),
],
)
def test_urls(self, url_name, url, view_class):
resolver = resolve(url)
assert resolver.view_name == url_name
assert resolver.func.view_class == view_class
Copy link
Member

Choose a reason for hiding this comment

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

This definitely handles the use case of giving people a sample to follow, but it would be more ideal to have a sample that 1. uses the database, and 2. tests something that is code we've written and not django code

Maybe we can create a new view that uses LoginRequiredMixin and test access that requires the use of factories and making database calls?

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.

Add starter test suite

3 participants