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

Update /tests/core/ #1105

Open
OTR opened this issue Jan 30, 2021 · 6 comments
Open

Update /tests/core/ #1105

OTR opened this issue Jan 30, 2021 · 6 comments

Comments

@OTR
Copy link

OTR commented Jan 30, 2021

The following is more like a feature request.

Unit tests in /tests/core/test_account.py::SignupViewTests doesnt match functional tests (actual behaviour of an app).

Description of a problem

Actual app (testproject) behaves as if AnonymousUser given as request.user, but tests cover CustomUser given as request.user.

My suggestion is to write tests for signing up of auth.models.AnonymousUser given as request.user because it is how the actual instance of wiki works. Currently tests doesnt cover this view. And put tests with CustomUser/ CustomGroup / VeryCustomUser into another directory. Something like down below:

--- a/tests/core/test_accounts.py
+++ b/tests/core/test_accounts.py
@@ -1,7 +1,8 @@
 from django.conf import settings as django_settings
+from django.conf import global_settings
 from django.contrib import auth
 from django.contrib.auth import authenticate
-from django.contrib.auth.models import AnonymousUser
+from django.contrib.auth.models import AnonymousUser, User
 from django.shortcuts import resolve_url
 from wiki.conf import settings as wiki_settings
 from wiki.models import reverse
@@ -107,9 +108,76 @@ class LoginTestViews(RequireRootArticleMixin, TestBase):
         self.assertIs(self.superuser1.is_authenticated, True)
         self.assertEqual(auth.get_user(self.client), self.superuser1)
 
-
+@wiki_override_settings(
+                        ACCOUNT_HANDLING=True,
+                        ACCOUNT_SIGNUP_ALLOWED=True,
+                        AUTH_USER_MODEL=global_settings.AUTH_USER_MODEL
+                        )
 class SignupViewTests(RequireRootArticleMixin, TestBase):
-    @wiki_override_settings(ACCOUNT_HANDLING=True, ACCOUNT_SIGNUP_ALLOWED=True)
+
+    def test_signup_passwd_too_similar(self):
+        response = self.client.post(
+            wiki_settings.SIGNUP_URL,
+            data={
+                "password1": "similar1",
+                "password2": "similar1",
+                "username": "similar1",
+                "email": "wiki@wiki.com",
+            },
+        )
+        errors = response.context_data["form"].errors
+        list_of_errors = list(errors.values())[0]
+        self.assertIn('The password is too similar to the username.', list_of_errors)
+
+    def test_signup_passwd_too_short(self):
+        response = self.client.post(
+            wiki_settings.SIGNUP_URL,
+            data={
+                "password1": "short",
+                "password2": "short",
+                "username": "wiki",
+                "email": "wiki@wiki.com",
+            },
+        )
+        errors = response.context_data["form"].errors
+        list_of_errors = list(errors.values())[0]
+        self.assertIn('This password is too short. It must contain at least 8 characters.', list_of_errors)
+
+    def test_signup_passwd_too_common(self):
+        response = self.client.post(
+            wiki_settings.SIGNUP_URL,
+            data={
+                "password1": "test",
+                "password2": "test",
+                "username": "wiki",
+                "email": "wiki@wiki.com",
+            },
+        )
+        errors = response.context_data["form"].errors
+        list_of_errors = list(errors.values())[0]
+        self.assertIn('This password is too common.', list_of_errors)
+
+    def test_signup(self):
+        response = self.client.post(
+            wiki_settings.SIGNUP_URL,
+            data={
+                "password1": "not-similar-and-short",
+                "password2": "not-similar-and-short",
+                "username": "wiki",
+                "email": "wiki@wiki.com",
+            },
+        )
+
+        self.assertIs(User.objects.filter(email="wiki@wiki.com").exists(), True)
+        self.assertRedirects(response, reverse("wiki:login"))

And make another sub-directory where placed tests that requires testadata app .

--- a/tests/custom/test_custom_accounts.py
+++ b/tests/custom/test_custom_accounts.py
+@wiki_override_settings(
+                        ACCOUNT_HANDLING=True,
+                        ACCOUNT_SIGNUP_ALLOWED=True,
+                        AUTH_USER_MODEL="testdata.CustomUser"
+)
+
+class SignupCustomUserViewTests(RequireRootArticleMixin, TestBase):
     def test_signup(self):
         response = self.client.post(
             wiki_settings.SIGNUP_URL,
@@ -120,5 +188,6 @@ class SignupViewTests(RequireRootArticleMixin, TestBase):
                 "email": "wiki@wiki.com",
             },
         )
+
         self.assertIs(CustomUser.objects.filter(email="wiki@wiki.com").exists(), True)
         self.assertRedirects(response, reverse("wiki:login"))

Also, it would be great if some documentation about testproject/settings/ and /tests/ provided on readthedocs, in order to easier get involved into contributing.

@benjaoming
Copy link
Member

benjaoming commented Jan 31, 2021

Hi @OTR

Thanks for opening this!

I dont know how to add a remote app

Can you explain in some simple way what you are trying to achieve? It's not clear for me what it is that is supposed to happen above, and you have supplied quite a lengthy set of changes and outputs :)

Brief background: The testproject is a bit pragmatic in its structure, not meant to be perfect. If it's of convenience, that's great.. if you want to contribute a few lines about how to achieve this "remote app" test design, please do.

https://django-wiki.readthedocs.io/en/master/development/index.html#get-started is always open for improvements.

@OTR OTR changed the title Update /tests/core/test_account.py Update /tests/core/ Jan 31, 2021
@OTR
Copy link
Author

OTR commented Jan 31, 2021

Can you explain in some simple way what you are trying to achieve?

I am trying to run tests from django-wiki/tests/ folder against my instance of wiki (which is modified testproject).

For now, I tried out tests from tests/core/ directory and I found out that tests that cover default behavior of wiki instance messed up with those that depend on testdata app and override default settings with custom ones. Moreover I found tests that depend on plugins ('attachments' and 'macros') but placed into core directory.

I ask you if it is possible to clean up tests directory and make it more organized.
It would be convenient:
If one wants to test only core functionality then one runs just pytest /tests/core;
If one wants to test only tests that override default settings then runs pytest /tests/custom;
if one wants to test plugins, then he knows that the all tests that cover plugins are placed in directory called plugins.

I moved some tests from core directory into others by their functionality, and here is a result:

├── core
│   ├── test_accounts.py
│   ├── test_basic.py
│   ├── test_checks.py
│   ├── test_commands.py
│   ├── test_forms.py
│   ├── test_managers.py
│   ├── test_markdown.py
│   ├── test_models.py
│   ├── test_paginator.py
│   ├── test_sites.py
│   ├── test_template_filters.py
│   ├── test_template_tags.py
│   ├── test_urls.py
│   ├── test_utils.py
│   └── test_views.py
├── custom
│   ├── test_custom_accounts.py
│   ├── test_custom_basic.py
│   ├── test_custom_checks.py
│   └── test_custom_views.py
├── plugins
│   ├── attachments
│   │   └── test_views.py
│   └── macros
│       └── test_toc.py

test_custom_accounts.py means all the tests from core/test_accounts.py that depends on CustomUser but moved in another directory called custom, so that in core directory only tests about dafault settings remained.

macros/test_toc.py - I put there tests that were in core directory but actually they dont work without wiki.plugin.macros in INSTALLED_APPS.

macros/test_views.py - I put there tests that were in core directory but actually they dont work without wiki.plugin.attachments in INSTALLED_APPS.

I just picked names of existed filenames that are look similar by their functionality.

I could provide you names of tests that depend on testdata app or plugins or make a fork with my replaced tests, if you are interested.

@benjaoming
Copy link
Member

I ask you if it is possible to clean up tests directory and make it more organized.

Yes, that's absolutely possible - you are more than welcome to contribute to improvements to how tests are organized for a better modular structure 👍

@OTR
Copy link
Author

OTR commented Feb 5, 2021

I made a branch with my changes. I split the changes into several commits to make it clear what exactly I did step by step.

https://github.com/OTR/django-wiki/tree/cleanup-tests-core

1. Move tests/core/test_models.py::ArticleModelTest::test_cache to tests/plugins/macros/test_models.py::ArticleModelTest::test_cache

Reason: requires wiki.plugins.macros.apps.MacrosConfig in INSTALLED_APPS

2. Move tests/core/test_template_tags.py::WikiRenderTest::test_called_with_preview_content_and_article_have_current_revision to tests/plugins/macros/test_template_tags.py::WikiRenderTest::test_called_with_preview_content_and_article_have_current_revision

Reason: requires wiki.plugins.macros.apps.MacrosConfig in INSTALLED_APPS

3. Move 'tests/core/test_template_filters.py::PluginEnabled::test_true to tests/plugins/attachments/test_template_filters.py::PluginEnabled::test_true

Reason: requires wiki.plugins.attachments.apps.AttachmentsConfig in INSTALLED_APPS

4. Move tests/core/test_accounts.py::SignupViewTests::test_signup to tests/custom/test_accounts.py::SignupViewTests::test_signup

Reason: requires testdata app

How to prove that it still runs without testdata app. Preparations:

Comment out the following lines in tests.settings:

# AUTH_USER_MODEL = "testdata.CustomUser"
# WIKI_GROUP_MODEL = "testdata.CustomGroup"
# ROOT_URLCONF = "tests.testdata.urls"
INSTALLED_APPS = [
	# "tests.testdata",

Add:

ROOT_URLCONF = "testproject.testproject.urls"

Run:

pytest --disable-warnings tests/core/test_accounts.py

All 5 tests passed

Revert changes in tests.settings and run:

$ pytest tests/custom/test_accounts.py

Both files are passed with original tests.settings

$ pytest tests/core/test_accounts.py tests/custom/test_accounts.py

5. Move tests/core/test_basic.py::CustomGroupTests to tests/custom/test_basic.py::CustomGroupTests

Reason: requires testdata app

6. Move test_check_for_fields_in_custom_user_model and test_custom_user_model_mitigation_required from tests/core/test_checks.py::CheckTests to tests/custom/test_checks.py::CheckTests

Reason: requires testdata app

7. Move tests/core/test_views.py::SettingsViewTests::test_change_group to tests/custom/test_views.py::SettingsViewTests::test_change_group

Reason: requires testdata app

Preparations:

Comment out the following lines in tests.settings:

# AUTH_USER_MODEL = "testdata.CustomUser"
# WIKI_GROUP_MODEL = "testdata.CustomGroup"
# ROOT_URLCONF = "tests.testdata.urls"

INSTALLED_APPS = [
	# "tests.testdata",

Add

ROOT_URLCONF = "testproject.testproject.urls"

This last step is needed to make core/test_views.py::SearchViewTest::test_heirarchy_search_404 passed. It fails on testproject's custom 404 view.

Comment out in testproject.testproject.urls

# handler500 = "testproject.views.server_error"
# handler404 = "testproject.views.page_not_found"

Then run pytest as above

  1. Move tests/core/test_sites.py to tests/custom/test_sites.py

Reason: requires testdata app

Conclusion:

Even if tests passed with DJANGO_SETTINGS_MODULE set as tests.settings (defined in pytest.ini) It is still a problem to make tests from tests/core folder run with testproject.testproject.settings set as DJANGO_SETTINGS_MODULE.

** I will update this topic later with my observations and outputs of pytest. Please don't close this issue yet.**

@OTR
Copy link
Author

OTR commented Feb 5, 2021

Found errors

1.

tests/core/test_views.py::SearchViewTest::test_hierarchy_search_404 fails on custom 404 view from testproject.testproject.views::page_not_found

with exception

django.template.exceptions.TemplateSyntaxError: Invalid block tag on line 16: 'endblocktrans', expected 'endblock'. Did you forget to register or load this tag?

A link to source code

def test_hierarchy_search_404(self):

> An example of how to reproduce the bug.

I added output.py just to make it highlighted when I open it in editor

$ cd testproject
$ pytest ../tests/core/test_views.py  --disable-warnings --ds=testproject.settings.base > output.py

Pytest output

OTR#2 (comment)

Narrowing the scope of a problem

When I comment out this line, the test is passed

handler404 = "testproject.views.page_not_found"

2.

Three tests from /tests/core/test_template_tags.py fails on TEMPLATES setting from testproject/testproject/settings/base.py

================================= short test summary info =================================
FAILED ../tests/core/test_template_tags.py::WikiFormTest::test_form_obj_is_baseform_instance
FAILED ../tests/core/test_template_tags.py::WikiRenderTest::test_called_with_preview_content_and_article_dont_have_current_revision
FAILED ../tests/core/test_template_tags.py::WikiRenderTest::test_if_preview_content_is_none

with exception:

django.template.exceptions.TemplateSyntaxError: You must enable the 'sekizai.context_processors.sekizai' template context processor or use 'sekizai.context.SekizaiContext' to render your templates.

Links to source code

def test_form_obj_is_baseform_instance(self):

def test_called_with_preview_content_and_article_dont_have_current_revision(self):

def test_if_preview_content_is_none(self):

> An example of how to reproduce the bug.

I added output.py just to make it highlighted when I open it in editor

$ cd testproject
$ pytest ../tests/core/test_template_tags.py  --disable-warnings --ds=testproject.settings.base > output.py

Pytest output

OTR#4 (comment)

Narrowing the scope of a problem

When I override that setting by adding the following line in testprojest/testproject/settings/local.py tests are passed:

TEMPLATES[0]["OPTIONS"]["debug"] = False

@benjaoming
Copy link
Member

Hi @OTR

Your contributions are most welcome, especially you are most welcome to contribute something on the line of what you had worked with in the first post:

My suggestion is to write tests for signing up of auth.models.AnonymousUser given as request.user because it is how the actual instance of wiki works. Currently tests doesnt cover this view. And put tests with CustomUser/ CustomGroup / VeryCustomUser into another directory. Something like down below:

If you are able to work out any subsequent issues, that would be great -- but that certainly isn't a requirement.

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

No branches or pull requests

2 participants