New test for another error case in DepositScriptAddress

On 14/09/14 22:15, Jimmy Song wrote:
> In waddrmgr/pool_test.go:
> 
>> +func TestDepositScriptAddressFailureToEncrypt(t *testing.T) {
>> +	tearDown, manager, pool := setUp(t)
>> +	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))
>> +		})
>> +
> 
> Is this really the best way to do this? It uses a callback in order to
> actually run the test, which isn't easy to see.

I think it's pretty easy to see that you're passing a function as an
argument, and the method's name starts with Run, which makes it pretty
obvious it's going to run the function passed to it.

> Something like Get/SetCryptoKeyScript in internal_test which gets/sets
> the mgr.cryptoKeyScript should be enough without all this callback
> business. This way the DepositScriptAddress function can be called
> cleanly after the set and error checked afterwards.

It may not be the best way, but IMO it's better than having the test
take care of mocking cryptoKeyScript and then restoring it after it's done.