-
Notifications
You must be signed in to change notification settings - Fork 6
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
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
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.
makes sense, I removed this and all instances you mentioned in 0b1637c
internal/services/ingest_test.go
Outdated
|
||
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)
internal/services/ingest_test.go
Outdated
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)},
})
internal/services/ingest_test.go
Outdated
|
||
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
internal/services/ingest_test.go
Outdated
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
internal/services/ingest_test.go
Outdated
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
internal/services/ingest_test.go
Outdated
|
||
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
internal/services/ingest_test.go
Outdated
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
internal/services/ingest_test.go
Outdated
|
||
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
internal/services/ingest_test.go
Outdated
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
internal/services/ingest_test.go
Outdated
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