chore(core): remove legacy "02" tests #3677
Conversation
vindard
force-pushed
the
remove-legacy-02-tests
branch
from
a53def1 to
b1cdbf8
Compare
vindard
force-pushed
the
remove-legacy-02-tests
branch
from
ee462a0 to
bb10581
Compare
core/api/test/legacy-integration/app/trigger/trigger.fn.spec.ts
Outdated
Show resolved
Hide resolved
| paymentRequest, | ||
| } | ||
|
|
||
| it("persists payment to be deleted in ln-payments collection", async () => { |
There was a problem hiding this comment.
How do you mean? There's a code branch in checkAndDeletePaymentForHash that persists an lnPayment if it didn't previously exist in LnPaymentsRepository that I explicitly wanted to test
There was a problem hiding this comment.
I review bottom -> up, there is no need to have 2 tests for this, is just add
const lnPaymentAfter = await lnPayments.findByPaymentHash(paymentHash)
if (lnPaymentAfter instanceof Error) throw lnPaymentAfter
expect(lnPaymentAfter.paymentHash).toBe(paymentHash)
in second test but it is up to you
There was a problem hiding this comment.
I'd prefer to have them separate because the 2nd test needs to have an explicit step that marks isCompleteRecord as true and I bring that in manually, and if I tried to not do that then the test becomes more complicated because later tests would rely on prior state in the test
| await lnPayments.persistNew({ | ||
| paymentRequest, | ||
| paymentHash, | ||
| sentFromPubkey: DEFAULT_PUBKEY, | ||
| }) | ||
| await lnPayments.update({ | ||
| paymentHash, | ||
| isCompleteRecord: true, | ||
| } as PersistedLnPaymentLookup) |
There was a problem hiding this comment.
this is what needs to be tested... deleteLnPaymentsBefore should persist it in lnpayment if it does not exists
There was a problem hiding this comment.
Ok I don't understand 😅 ... isn't that what this tests
There was a problem hiding this comment.
is this not done by await Lightning.deleteLnPaymentsBefore(timestamp2Months) line 89?
There was a problem hiding this comment.
Ah nope, it persists it but then it won't get isCompleteRecord property marked as true unless we run updateLnPayments which is a lot more than we need just to test that deletePaymentByHash gets hit under the right conditions
So I just manually setup the lnPayment in the repository so that the use-case can continue on to what we want to test here
| @@ -104,6 +122,9 @@ describe("onchainBlockEventHandler", () => { | |||
| let lastHeight = initialBlock | |||
| const subBlocks = subscribeToBlocks({ lnd: lnd1 }) | |||
| subBlocks.on("block", async ({ height }: { height: number }) => { | |||
| // this sleep seems necessary to allow confirmations to propagate for | |||
| // 'getChainTransactions' call inside 'listIncomingOnChainTransactions' | |||
| await sleep(500) | |||
There was a problem hiding this comment.
then is something else.... please dont use this inside the handler (we dont have this in trigger handler)
There was a problem hiding this comment.
Ok removed here, will troubleshoot again when we're actually removing 01 folder be614f5
There was a problem hiding this comment.
Oh actually, we already have this test in bats #3710
vindard
force-pushed
the
remove-legacy-02-tests
branch
from
be614f5 to
ef1e61b
Compare
No description provided.