Skip to content

Wallet test fixes #280

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

Merged
merged 2 commits into from
Sep 9, 2014
Merged

Wallet test fixes #280

merged 2 commits into from
Sep 9, 2014

Conversation

dcousens
Copy link
Contributor

@dcousens dcousens commented Sep 8, 2014

As per the suggestion in #247, this PR removes the use of sinon in Transaction and instead tests that the Transaction is exactly as it should be since the hash is deterministic.

It also makes a few fixes thanks to jshint and re-works a test in Wallet which was not testing the desired behaviour.

@dcousens dcousens changed the title Testprefix Wallet test fixes Sep 8, 2014
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 166f2c4 on dcousens:testprefix into f028dff on bitcoinjs:master.

var fee = 14570
var tx = wallet.createTx(to, value)
assert.equal(tx.outs.length, 1)
var tx1 = wallet.createTx(to, value - 546)
Copy link

Choose a reason for hiding this comment

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

I assume this is just a clean-up and unrelated to the kill sinon specific change?
Edit: Just read your commit comment where you mentioned it :) Cool

@glorat
Copy link

glorat commented Sep 8, 2014

+1 like

@dcousens
Copy link
Contributor Author

dcousens commented Sep 8, 2014

We should probably still isolate the issue with sinon before shrugging it off though.

@dcousens
Copy link
Contributor Author

dcousens commented Sep 9, 2014

Removed sinon-removing commit. Turns out the problem was very obscure, but none the less a fault in the test logic.
The lazy evaluation of ecurve.Point._zInv was causing a deep equality to fail when getPrivateKeyForAddress was freshly deriving a new key pair.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c3f8869 on dcousens:testprefix into f028dff on bitcoinjs:master.

weilu added a commit that referenced this pull request Sep 9, 2014
@weilu weilu merged commit 32cbd02 into bitcoinjs:master Sep 9, 2014
@dcousens dcousens deleted the testprefix branch September 9, 2014 10:58
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.

4 participants