services/horizon: Experiment with stub-based testing instead of mock-based

[paulbellamy]

What problem does your feature solve?

Mock-based tests can be quite fragile and difficult to maintain. e.g. making a change, and having to update mocks in lots of unrelated tests. Stubs can help alleviate that.

What would you like to see?

Try a stub-based approach for a couple tests. Identify a few good tests to start with. Ingest processor ones using a historyQ could be a good candidate.

What alternatives are there?

• Leave it as is
• Try other approaches
• try to improve our mocking usage

[bartekn]

Can you elaborate? AFAIR, in most cases we use mocks because we actually check if the arguments are correct.

[paulbellamy]

But, a lot of the time those specific argument values are unrelated to the given test (at least for the ingest processors I've seen). Most of our tests are more concerned with what the resulting state/output is, vs how it got there. For tests specifically concerned with internal calls of how a thing happens mocks definitely make more sense. But, as it is we have to mock every interaction with the DB, which obscures the actual intent of the test. Hopefully, we could avoid a bunch of excess mocking of internal implementation details, for example NewTransactionClaimableBalanceBatchInsertBuilder.

One experiment I was thinking was replacing the mocks with a test-only in-memory implementation, e.g. maybe something like

type MockQAssetStats struct {
  data map[string]ExpAssetStat
}

// rest of the implementation to fulfill the QAssetStats interface

So then for an asset-stat-update test you could:

// Arrange
q := NewMockQAssetStats()
q.InsertAssetStats(/*some pre-existing test fixtures (or randomly generated)*/)
processor := NewAssetStatProcessor(q)

// Act
processor.ProcessChange(/* some change updating an asset stat */)

// Assert
result, err := q.GetAssetStat(/* ... */)
assert.NoError(t, err)
assert.Equal(t, expected, result)

The rough plan here would be to pick a couple tests, then see if this simplifies things, or not...

[bartekn]

But, a lot of the time those specific argument values are unrelated to the given test (at least for the ingest processors I've seen).

In such case we use mock.Anything or mock.AnythingOfType(type). There are some examples in ingest package.

I agree that in-memory implementation of some interfaces makes sense because it will simplify the test code.

[paulbellamy]

mock.Anything is alright, but we still have to mock the calls. This is, I guess, the other option, of "try improving our mock usage", as maybe there's a way to only specify the mock calls we care about more precisely.

On Wed, Mar 31, 2021 at 5:44 PM Bartek Nowotarski @.***> wrote:

But, a lot of the time those specific argument values are unrelated to the given test (at least for the ingest processors I've seen).

In such case we use mock.Anything or mock.AnythingOfType(type). There are some examples in ingest package.

I agree that in-memory implementation of some interfaces makes sense because it will simplify the test code.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stellar/go/issues/3519#issuecomment-811237836, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5CV4HPVNU5GJAG46ACHTTGNGNBANCNFSM42ES44LA .