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

Tests for template filters. #382

Merged
merged 5 commits into from
Feb 8, 2015
Merged

Tests for template filters. #382

merged 5 commits into from
Feb 8, 2015

Conversation

Alkalit
Copy link
Contributor

@Alkalit Alkalit commented Feb 4, 2015

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.34%) to 57.24% when pulling 43ac168 on Alkalit:master into 6daab12 on django-wiki:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.34%) to 57.24% when pulling 43ac168 on Alkalit:master into 6daab12 on django-wiki:master.

@benjaoming
Copy link
Member

Thanks for this great improvement!

Would you consider using Django's own builtin decorator for purposes of changing settings? It can avoid mock as a dependency.... but to be honest, I've been part of a project where mock ruined all the tests because it was used for everything :)

https://docs.djangoproject.com/en/1.7/topics/testing/tools/#django.test.SimpleTestCase.settings

@Alkalit
Copy link
Contributor Author

Alkalit commented Feb 4, 2015

Well, I'll try to rewrite.

@Alkalit
Copy link
Contributor Author

Alkalit commented Feb 6, 2015

Now it's my turn to be honest - I used mock because could not achieve the same result by override decorator. For the tests I need to change the settings for the module wiki.core.permissions, but somehow built-in decorator does not work. But if in the same module to import settings directly from django.conf.settings, it has correct values. Unfortunately I do not know how to fix it.

P.S. I also create question on SO.

@tkliuxing
Copy link
Contributor

override_settings function just send a signal django.test.signals.setting_changed.
If not register callbacks function, the settings item doesn't change.

Forgive me in broken English ;-)

@tkliuxing
Copy link
Contributor

Maybe you can do this:

from wiki.conf import settings

class CanRead(BaseTestCase):

    template = """
        {% load wiki_tags %}
        {{ article|can_read:user }}
    """
    def test_user_have_permission(self):
        settings.CAN_READ = lambda *args: True
        a = Article.objects.create()

        User = get_user_model()
        u = User.objects.create(username='Nobody', password='pass')

        output = can_read(a, u)
        self.assertTrue(output)

        output = self.render(self.template, {'article': a, 'user': u})
        self.assertIn('True', output)

@Alkalit
Copy link
Contributor Author

Alkalit commented Feb 6, 2015

@tkliuxing thanks for assistance, but your example seems like reinvent the wheel. It breaks tests isolation, which mean for each test should be written something like this

def setUp(self):
    super(CanRead, self).setUp()

    self.default_value = getattr(settings, 'CAN_READ', None)

def tearDown(self):
    super(CanRead, self).tearDown()
    settings.CAN_READ = self.default_value

which can be pretty ugly and not convenient if need few settings.

Meybe in some step you will invent context manager. Next step is decorating.

P.S. Oh, believe me - my english even worse :D

@benjaoming
Copy link
Member

@Alkalit
Have a look at this line at the top of the test:

from wiki.models import URLPath

This will trigger load-time events in the whole wiki application, I think this is what breaks the settings decorator because settings are then initialized before the test is run.

Try moving all import statements interacting with wiki to your test methods, thereby they should be able to override settings and it also improves your isolation, I think..... looking forward to hear if you find out more about this, to be honest I never really thought about this level of test isolation.

@Alkalit
Copy link
Contributor Author

Alkalit commented Feb 7, 2015

And winner in nomination workaround of the year is... ta-daaah:

@override_settings(WIKI_CAN_READ=lambda *args: False)
def test_user_dont_have_permission(self):

    from imp import reload
    from wiki.conf import settings
    reload(settings)

    User = get_user_model()

    a = Article.objects.create()
    u = User.objects.create(username='Noman', password='pass')

    output = can_read(a, u)
    self.assertFalse(output)

    output = self.render(self.template, {'article': a, 'user': u})
    self.assertIn('False', output)

Tested it in few methods. Seems working. What do you think?
Also @benjaoming thank for advice.

@Alkalit
Copy link
Contributor Author

Alkalit commented Feb 7, 2015

And second place is awarded to:

class wiki_override_settings(override_settings):

    def __enter__(self):
        super(wiki_override_settings, self).__enter__()

        from imp import reload
        from wiki.conf import settings
        reload(settings)

@coveralls
Copy link

Coverage Status

Coverage increased (+1.94%) to 57.84% when pulling c43c971 on Alkalit:master into 6daab12 on django-wiki:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.94%) to 57.84% when pulling c43c971 on Alkalit:master into 6daab12 on django-wiki:master.

benjaoming added a commit that referenced this pull request Feb 8, 2015
Tests for template filters.
@benjaoming benjaoming merged commit 9b8be37 into django-wiki:master Feb 8, 2015
@benjaoming
Copy link
Member

obliged!

@benjaoming
Copy link
Member

and thanks for finding a solution to settings overrides!

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.

None yet

4 participants