feat(SPV-1059) fee and change #910
Conversation
Manual TestsβΉοΈ Remember to ask team members to perform manual tests and to assign |
| ) | ||
|
|
||
| type TransactionDetailsAssertions interface { | ||
| WithOutValues(values ...bsv.Satoshis) TransactionDetailsAssertions |
There was a problem hiding this comment.
Is it without or with output?
There was a problem hiding this comment.
I'll rename it to WithOutputValues
| } | ||
|
|
||
| func (a *transactionAssertions) WithOutValues(values ...bsv.Satoshis) TransactionDetailsAssertions { | ||
| if len(values) != len(a.tx.Outputs) { |
There was a problem hiding this comment.
Please add t.Helper() - I recently add it in many places, it's possible that I overlooked something but still maybe it's better to use this approach in new methods, and add missing when we accidentally came across such a method
| if len(values) != len(a.tx.Outputs) { | ||
| a.t.Fatalf("expected %d outputs, got %d", len(values), len(a.tx.Outputs)) | ||
| } |
There was a problem hiding this comment.
To follow some convention that we are using require in many packages I would rather use it here like:
| if len(values) != len(a.tx.Outputs) { | |
| a.t.Fatalf("expected %d outputs, got %d", len(values), len(a.tx.Outputs)) | |
| } | |
| a.require.Lenf(a.tx.Outputs, len(values), "Tx has less outputs then expected values") |
|
|
||
| // and: | ||
| client := given.HttpClient().ForUser() | ||
| for variationName, variation := range variations { |
There was a problem hiding this comment.
I know that previously there was a duplication and this wasn't best solution,
But this isn't best either.
Maybe it's just a mistake in our thinking.
Maybe we should have all those different cases for default request.
And simply 2 additional tests for different format where we will check only the format of hex.
It should be clearer then, and easier to run particular thing.
There was a problem hiding this comment.
I've just introduced new approach.
I tried not to repeat test-cases not testing code.
Take a look π€
for name, test := range successTestCases {
t.Run(name, func(t *testing.T) {
suite.Run(t, newTxOutlineSuite(test, initialSatoshis))
})
}
basicTestCase := successTestCases["create transaction outline for op_return strings as default data"]
t.Run("explicit beef as requested format", func(t *testing.T) {
suite.Run(t,
newTxOutlineSuite(basicTestCase, initialSatoshis).withExplicitFormat("BEEF"),
)
})
t.Run("explicit raw as requested format", func(t *testing.T) {
suite.Run(t,
newTxOutlineSuite(basicTestCase, initialSatoshis).withExplicitFormat("RAW"),
)
})| @@ -24,6 +24,11 @@ func (t *TransactionSpec) evaluate(ctx *evaluationContext) (*sdk.Transaction, tr | |||
| return nil, transaction.Annotations{}, err | |||
| } | |||
|
|
|||
| outputs, err = calculateChange(ctx, inputs, outputs, ctx.feeUnit) | |||
| if err != nil { | |||
| return nil, transaction.Annotations{}, spverrors.Wrapf(err, "failed to calculate change") | |||
There was a problem hiding this comment.
Maybe it would be better to have some defined error here?
engine/v2/transaction/outlines/testabilities/fixture_outline_transaction.go
Outdated
Show resolved
Hide resolved
# Conflicts: # actions/v2/transactions/internal/mapping/outline_tx.go # models/transaction/annotated_transaction.go
Co-authored-by: Krzysztof Tomecki <[email protected]>
# Conflicts: # actions/testabilities/assert_transactions_endpoint.go # engine/client_internal.go # engine/v2/transaction/outlines/interface.go # engine/v2/transaction/outlines/testabilities/fixture_outline_transaction.go # engine/v2/transaction/outlines/transaction_outlines_service.go # engine/v2/transaction/outlines/utxo/internal/sql/inputs_query_composer.go # engine/v2/transaction/outlines/utxo/internal/sql/selector_example_test.go
Pull Request Checklist