Skip to content

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.

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