-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[services][feat] #81: Add test for ingesting path payments #84
base: main
Are you sure you want to change the base?
Conversation
internal/services/ingest_test.go
Outdated
@@ -215,3 +216,166 @@ func TestIngestPayments(t *testing.T) { | |||
assert.Equal(t, payments[0].TransactionHash, "abcd") | |||
}) | |||
} | |||
|
|||
func TestIngestPathPaymentsSend(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make this a unit test under the TestIngestPayments function? With a t.Run("op_pat_payment_send", func(t *testing.T) { ...} ? That way you can also reuse the same set up code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, good call. Updated in 90f7d32
internal/services/ingest_test.go
Outdated
}) | ||
} | ||
|
||
func TestIngestPathPaymentsReceive(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. Can you make this a unit test under the TestIngestPayments function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in 90f7d32
|
||
t.Run("test_op_path_payment_send", func(t *testing.T) { | ||
err := models.Account.Insert(context.Background(), srcAccount) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking here but I don't really think you require this line. This is not a test to see if models are working. I'd replace it with _ := models.Account.Insert(context.Background(), srcAccount)
Operations: []txnbuild.Operation{&pathPaymentOp}, | ||
Preconditions: txnbuild.Preconditions{TimeBounds: txnbuild.NewTimeout(10)}, | ||
}) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above. This can be replaced with:
transaction, _ := txnbuild.NewTransaction(txnbuild.TransactionParams{
SourceAccount: &txnbuild.SimpleAccount{
AccountID: keypair.MustRandom().Address(),
},
Operations: []txnbuild.Operation{&pathPaymentOp},
Preconditions: txnbuild.Preconditions{TimeBounds: txnbuild.NewTimeout(10)},
})
|
||
signer := keypair.MustRandom() | ||
errSigner := models.Account.Insert(context.Background(), signer.Address()) | ||
require.NoError(t, errSigner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
require.NoError(t, errSigner) | ||
|
||
signedTx, err := transaction.Sign(network.TestNetworkPassphrase, signer) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
require.NoError(t, err) | ||
|
||
txEnvXDR, err := signedTx.Base64() | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
|
||
t.Run("test_op_path_payment_receive", func(t *testing.T) { | ||
err := models.Account.Insert(context.Background(), srcAccount) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
Operations: []txnbuild.Operation{&pathPaymentOp}, | ||
Preconditions: txnbuild.Preconditions{TimeBounds: txnbuild.NewTimeout(10)}, | ||
}) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
|
||
signer := keypair.MustRandom() | ||
errSigner := models.Account.Insert(context.Background(), signer.Address()) | ||
require.NoError(t, errSigner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
require.NoError(t, errSigner) | ||
|
||
signedTx, err := transaction.Sign(network.TestNetworkPassphrase, signer) | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
require.NoError(t, err) | ||
|
||
txEnvXDR, err := signedTx.Base64() | ||
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
Closes #81
What
Add test for ingest of path payments
Why
Current test cases only cover regular payments
Known limitations
N/A
Issue that this PR addresses
#81
PR Structure
all
if the changes are broad or impact many packages.Thoroughness
Release