feat(SPV-1169) testabilities for record transaction outline #761
Conversation
Manual TestsβΉοΈ Remember to ask team members to perform manual tests and to assign |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #761 +/- ##
==========================================
- Coverage 45.46% 45.16% -0.31%
==========================================
Files 339 344 +5
Lines 17416 17547 +131
==========================================
+ Hits 7919 7925 +6
- Misses 8906 9033 +127
+ Partials 591 589 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
dorzepowski
changed the base branch from
main
to
feat/SPV-1169-mock-beef-gen
chris-4chain
force-pushed
the
feat/SPV-1169-testabilities
branch
from
f34bbf7
to
7581865
Compare
| transactions: make(map[string]database.Transaction), | ||
| outputs: make(map[string]database.Output), | ||
| data: make(map[string]database.Data), | ||
| } | ||
| } | ||
|
|
||
| func (m *mockRepository) SaveTX(_ context.Context, txTable *database.Transaction, outputs []*database.Output, data []*database.Data) error { | ||
| func (m *MockRepository) SaveTX(_ context.Context, txTable *database.Transaction, outputs []*database.Output, data []*database.Data) error { |
There was a problem hiding this comment.
Maybe some testabilities like:
WillFailOnSaveTX as a valuable test case? + such example in record_outline_test.go?
There was a problem hiding this comment.
Good idea
I'm adding this WillFailOnSaveTX and a test case for that π
| @@ -36,7 +35,7 @@ func (m *mockRepository) SaveTX(_ context.Context, txTable *database.Transaction | |||
| return nil | |||
| } | |||
|
|
|||
| func (m *mockRepository) GetOutputs(_ context.Context, outpoints iter.Seq[bsv.Outpoint]) ([]*database.Output, error) { | |||
| func (m *MockRepository) GetOutputs(_ context.Context, outpoints iter.Seq[bsv.Outpoint]) ([]*database.Output, error) { | |||
There was a problem hiding this comment.
And the same proposition here (but maybe it's too deep) to have WillFailOnGetOutputs? π€
There was a problem hiding this comment.
No problem π
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestOnBroadcastErr(t *testing.T) { |
There was a problem hiding this comment.
What do you think about more complex test cases like multiple error handling?
Example / pseudo code:
func TestRecordOutlineMultipleFailures(t *testing.T) {
// given:
given, then := testabilities.New(t)
service := given.
WithStoredOutputs(database.Output{
TxID: givenTXWithOpReturn(t).InputUTXO(0).TxID,
Vout: givenTXWithOpReturn(t).InputUTXO(0).Vout,
SpendingTX: nil,
}).
WillFailOnSaveTX(errors.New("repository save error")).
WillFailOnBroadcast(errors.New("broadcaster error")).
NewRecordService()
// and:
outline := &outlines.Transaction{
BEEF: givenTXWithOpReturn(t).BEEF(),
Annotations: transaction.Annotations{
Outputs: transaction.OutputsAnnotations{
0: &transaction.OutputAnnotation{
Bucket: bucket.Data,
},
},
},
}
// when:
err := service.RecordTransactionOutline(context.Background(), outline)
// then:
then.WithErrorIs(err, txerrors.ErrTxRepositorySave).NothingChanged()
then.WithErrorIs(err, txerrors.ErrTxBroadcast).NothingChanged()
}There was a problem hiding this comment.
That won't work like you're showing.
The error can be either ErrTxBroadcast or ErrTxRepositorySave - but not both at the same time.
Taking this into account - I don't think, such test will be beneficial.
| WithNoError(err error) SuccessfullyCreatedRecordOutlineAssertion | ||
| WithErrorIs(err, expectedError error) ErrorAssert | ||
|
|
||
| StoredOutputs([]database.Output) RecordOutlineAssert |
There was a problem hiding this comment.
Just a thought, don't have example π
What do you think about checking if the final "repository" state matches exactly what we intended without any additional entries?
There was a problem hiding this comment.
I mean something like:
then.WithNoError(err).
Broadcasted(givenTXWithOpReturn(t).ID()).
StoredOutputs(expectedOutputs).
StoredData(expectedData)actualOutputs := given.repository.GetAllOutputs()
actualData := given.repository.GetAllData()
require.ElementsMatch(t, expectedOutputs, actualOutputs, "Repository contains unexpected outputs")
require.ElementsMatch(t, expectedData, actualData, "Repository contains unexpected data")And at the top define some expectedData used here as a parameter for require π
There was a problem hiding this comment.
I've changed from Subset to ElementsMatch, so now the behawior is as you described.
This PR waits for #759
Pull Request Checklist