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

Unittest conversion: ejtp.crypto ($20) #90

Closed
MaddieM4 opened this issue Feb 13, 2013 · 8 comments
Closed

Unittest conversion: ejtp.crypto ($20) #90

MaddieM4 opened this issue Feb 13, 2013 · 8 comments

Comments

@MaddieM4
Copy link
Owner

Part of the #45 unittest conversion process. Create unittest equivalents for the doctests currently in the ejtp.crypto package. There are surprisingly few tests to convert, which is why the whole package is being treated as a single issue.

@MaddieM4
Copy link
Owner Author

Bounty is $20.

http://www.freedomsponsors.org/core/issue/179/unittest-conversion-ejtpcrypto


(Copied from acceptance criteria)

Provide equivalent test coverage to what already exists as doctests. Remove any doctests you feel do not contribute to actual documentation.

@MaddieM4
Copy link
Owner Author

Merged via #93.

@MaddieM4
Copy link
Owner Author

Turns out this broke stuff, and I only didn't notice because some local modules were cached. After clearing my cache, it fails locally in the same way as Travis. I'm diagnosing it now, but it's weird.

@MaddieM4
Copy link
Owner Author

This would be so damn simple to solve if RawData could actually be accurately instantiated the same way it repr's, like one would be lead to believe. Turns out that's too big a problem to solve for the sake of a patch. But make no mistake, that will be dealt with. Either the __init__ function changes, or we change the __repr__ function, but one of those has to move.

Looks like the hard solution just became the easy one. Reverting my local changes and trying again. I love when both test mechanisms utterly barf out and I have to stay up late fixing the primary branch of development, that's fun and a half. Need some streamers up in here.

@MaddieM4
Copy link
Owner Author

This ain't happening tonight. There's just no way. I'm going to have to revert the merge, create the ticket that will make this feasible, get that done, and remerge. Sorry, @iurisilvio. Don't worry, I'll handle this, I'm just really tired and slightly punchy.

@MaddieM4 MaddieM4 reopened this Feb 14, 2013
@iurisilvio
Copy link
Contributor

Interesting... It happened because I did a small change to make things work easily. :/

@MaddieM4
Copy link
Owner Author

Well, don't worry about it, you basically exposed a flaw in RawData's
design that I have to fix anyway.
On Feb 14, 2013 2:39 AM, "Iuri de Silvio" notifications@github.com wrote:

Interesting... It happened because I did a small change to make things
work easily. :/


Reply to this email directly or view it on GitHubhttps://github.com//issues/90#issuecomment-13541823.

@MaddieM4
Copy link
Owner Author

Okay, this is done and merged. I really do love the cross-version peace of mind that the new RawData repr gives us, we're probably gonna wanna do that in more places too. But it can wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants