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

Add tests as data files #2

Closed
wants to merge 1 commit into from
Closed

Conversation

perrito666
Copy link
Contributor

Tests file provides a client implementation that works with the non standard sessionengine, they need to be added to the data files so they are provided for third party users when they try to write tests while using this module

@Bouke
Copy link
Collaborator

Bouke commented Jan 6, 2014

If I understand this PR correctly, this will include the tests/ files under user_sessions/tests? So you could do from user_sessions.tests.tests import Client to use the custom test client?

@perrito666
Copy link
Contributor Author

correct, since your custom client is required to run tests when user
user_sessions.

On Mon, Jan 6, 2014 at 7:21 PM, Bouke Haarsma notifications@github.comwrote:

If I understand this PR correctly, this will include the tests/ files
under user_sessions/tests? So you could do from user_sessions.tests.tests
import Client to use the custom test client?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-31693768
.

Horacio Durán

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d68cd6a on perrito666:master into 6789d7d on Bouke:master.

@Bouke
Copy link
Collaborator

Bouke commented Jan 6, 2014

Ah okey, thanks. That would be useful to add indeed! I will investigate the effects of adding the code. Could you stash the commits into a single commit?

@perrito666
Copy link
Contributor Author

sure, I will do it tomorrow :) I am seeking the best way to do it since
this works when running python setup.py install but not when doing pip
install gitrepo... perhaps a bug in pip

On Mon, Jan 6, 2014 at 7:27 PM, Bouke Haarsma notifications@github.comwrote:

Ah okey, thanks. That would be useful to add indeed! I will investigate
the effects of adding the code. Could you stash the commits into a single
commit?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-31694275
.

Horacio Durán

@Bouke
Copy link
Collaborator

Bouke commented Jan 6, 2014

Ah okey, yes. Maybe the Client should be separated from the actual tests. Maybe some sort of utils module that contains test helpers. Then no setup.py trickery would be needed.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.12%) when pulling 1838e69 on perrito666:master into 6789d7d on Bouke:master.

@perrito666
Copy link
Contributor Author

Here it makes more sense this way, client is as an util for tests, it looks indeed cleaner.
I also added a small doc to the client class to explain why is it necessary


class Client(BaseClient):
"""
Custom implementation of django.test.Client, it is required to perform
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you start with a single line explanation first? Like this:

"""
Custom implementation of django.test.Client

It is required to perform tests that require to login in sites using 
django-user-sessions since its implementation of SessionStore has to 
required parameters which is not in concordance with what is expected 
from the original Client.
"""

@Bouke
Copy link
Collaborator

Bouke commented Jan 7, 2014

Looks good to me. If you could make the documentation change and squash the commits, I'll merge it in!

…t is

available to users installing the egg only
* Changed comments format to suit the project's requirements
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.12%) when pulling 10f3f13 on perrito666:master into 6789d7d on Bouke:master.

@perrito666
Copy link
Contributor Author

There, I think it looks nice now, it follows the formatting of the rest and has been squashed into one commit

@Bouke Bouke closed this in 99dba5b Jan 7, 2014
@Bouke
Copy link
Collaborator

Bouke commented Jan 7, 2014

Thanks for your contribution, I appreciate your effort! 👍

@perrito666
Copy link
Contributor Author

We are using your module and it helps a lot, it is the least I could do,
thank you.

On Tue, Jan 7, 2014 at 12:26 PM, Bouke Haarsma notifications@github.comwrote:

Thanks for your contribution, I appreciate your effort! [image: 👍]


Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-31746502
.

Horacio Durán

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

3 participants