New test for another error case in DepositScriptAddress

On 15/09/14 10:49, Jimmy Song wrote:
> In waddrmgr/pool_test.go:
> 
>> +	defer tearDown()
>> +	if err := pool.CreateSeries(0, []string{pubKey0, pubKey1, pubKey2}, 2); err != nil {
>> +		t.Fatalf("Cannot creates series")
>> +	}
>> +
>> +	var err error
>> +	// Replace pool.manager.cryptoKeyScript with a fake one that has an Encrypt()
>> +	// method that always errors out. That should be caught and propagated as
>> +	// an ErrCrypto by DepositScriptAddress.
>> +	waddrmgr.RunWithReplacedCryptoKeyScript(
>> +		manager, &FailingToEncryptCryptoKey{}, func() {
>> +			_, err = pool.DepositScriptAddress(0, 0, uint32(1))
>> +		})
>> +
>> +	checkManagerError(t, "", err, waddrmgr.ErrCrypto)
>> +}
> 
>     Line 411 in pool.go is not a return statement:
> 
> Sorry, you might have seen this before I changed that to say "insert".
> If you return nil, nil, basically, your test passes when it's looking
> for this function to fail.

A "return nil, nil" on line 411 does not affect my test, and that is
expected.

> 
>     What I meant about testName is that it's useful when you have
>     table-driven tests, but redundant otherwise.
> 
> Just looked through manager_test.go. There's never a time when testName
> is an empty string. If you're going to use their function, you should
> use it the way they use it.

Because they have a single test function (of the TestXxx(t *testing.T)
form) that runs all the separate checks they want, so without the
testName argument the failure output would only show TestXxx and they'd
have a hard time figuring out what specific test failed. In our case we
have a specific test with a single call to checkManagerError() so when
the test fails it's obvious what check failed and we don't need to
duplicate that information