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 to client, address and identity #77

Merged
merged 3 commits into from Feb 10, 2013

Conversation

Projects
None yet
2 participants
@iurisilvio
Copy link
Contributor

commented Feb 10, 2013

I started moving identity tests and decided to do #70, #71 and #72 in a pull request.

Moved some doctests, deleted some code, added a lot of unittests.

@@ -0,0 +1,61 @@
import logging
try:

This comment has been minimized.

Copy link
@campadrenalin

campadrenalin Feb 10, 2013

Owner

Something we should perhaps move into ejtp.util.compat?

@campadrenalin

This comment has been minimized.

Copy link
Owner

commented Feb 10, 2013

Took forever to look over since it was all one pull request, but this all checks out. I did put in a recommendation that we should add StringIO to ejtp.util.compat, but that can wait for another day.

@campadrenalin campadrenalin merged commit a00aeea into campadrenalin:development Feb 10, 2013

1 check passed

default The Travis build passed
Details

@iurisilvio iurisilvio deleted the iurisilvio:more_tests branch Feb 11, 2013

@iurisilvio

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2013

Most changes were related, so I decided to make it only one pull request.

I decided to not put the StringIO in compat module because I'm still not confortable with a big compat module. Nevertheless, I don't have a better way to maintain compatibility yet.

@campadrenalin

This comment has been minimized.

Copy link
Owner

commented Feb 11, 2013

Alright. Well, we can always change it later if we need it again elsewhere. I suspect we will make more common use of it thanks to logging-based tests, but no need to jump on that preemptively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.