Some transaction tests were not run…

[dskloet]

Some transaction tests were not run because of the way they were built. The builder was invoked, but the built test wasn't. Fixed this so they are run and fixed one test that still assumed the dust limit at 5460 instead of 546.

[coveralls]

Coverage remained the same at 97.79% when pulling e294505 on dskloet:test/fix-transaction into bb843ee on bitpay:master.

[dskloet]

The reason the test coverage didn't increase is because there are some ifs that used to only take the true branch and now they also take the false branch, but that doesn't increase the number of lines/statements covered because they don't have an else block. Pull request #1226 restructured these ifs which uncovered the missing coverage.

[braydonf]

Good catch. We may want to run these instead by doing:

it('can skip the check for too much fee', function() {
  buildSkipTest(function(transaction) {
    //...
  }, 'disableLargeFees')();
});

[dskloet]

Why? buildSkipTest already returns a function. The extra wrapper doesn't add anything except another stack frame, and (apparently) an easy way to hide when a test is not run. Or am I missing something?

[braydonf]

Right, build returns a function (and without a first argument).

[dskloet]

Which first argument?

Another option is to rename buildSkipTest to doSkipTest and make it actually perform the test instead of return a function. Then I just invoke that from the actual test. What do you think?

[braydonf]

I think it's good the way you have it now.