Skip to content

Conversation

@dcousens
Copy link
Contributor

As can be seen after a small commit to just the README, the build has switched from passing to failed.

The error is an Uncaught AssertionError: 18600000 == 18500000, that is, the funds received on that address were more than expected.
Knowing that the assertion code is actually:

assert.equal(resource.balance, startingBalance + 100000)

It received twice as much as it should. This indicates that the code was run twice before the assertion was made.

Looking at the transaction history, it can be seen that the two transactions around this time were performed asynchronously.

To resolve this, I have made the following changes:

  • targetAddress is now a random address.
  • No need to check targetAddress balance, if we regenerate an address with a non-zero balance, a failing integration test would be the least of our worries.
  • Decreased the number of Satoshi's by a magnitude of 10 (now 0.0001BTC).
  • Cleaned up the integration test some (formatting and clearer comments etc), and
  • Don't withdraw from the faucet every time.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling 0392425 on dcousens:intasync into 4332c9e on bitcoinjs:master.

@dcousens
Copy link
Contributor Author

Should be ready to go now.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 5e27061 on dcousens:intasync into 4332c9e on bitcoinjs:master.

@kyledrake
Copy link
Contributor

+1

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling dfe1034 on dcousens:intasync into 4332c9e on bitcoinjs:master.

@weilu
Copy link
Contributor

weilu commented May 28, 2014

TBH, I'm not a big fan of how much this PR complicates the simple integration test. I'm in favor of using random addresses to address the race condition. But I'd rather drop the fee + min spendable calculation stuff. Just draw an small amount from faucet every time. We can return the amount back to faucet with an afterEach.

@dcousens
Copy link
Contributor Author

@weilu I cleaned it up as per your request. We can't easily do an afterEach unless we have a deterministic way of determining which key we used last time (or we wait for a confirmation so we can propagate a follow up transaction).

While I think this pull request is still worthwhile, I've realised there still exists a race condition over which unspent is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot simply throw here, need to call done(), otherwise on error the test will timeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3287f73 on dcousens:intasync into 778aab1 on bitcoinjs:master.

weilu added a commit that referenced this pull request May 28, 2014
Integration tests running async causing wrong balance
@weilu weilu merged commit c9ad359 into bitcoinjs:master May 28, 2014
@dcousens dcousens deleted the intasync branch May 28, 2014 16:12
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