[CCFPCM-419] pos reconciliation service and pos deposit service unit tests#101
[CCFPCM-419] pos reconciliation service and pos deposit service unit tests#101chelsea-EYDS merged 2 commits intomainfrom
Conversation
chelsea-EYDS
changed the title
chelsea-EYDS
force-pushed
the
CCFPCM-419
branch
2 times, most recently
from
e970d6c to
2cb4b72
Compare
chelsea-EYDS
force-pushed
the
CCFPCM-419
branch
4 times, most recently
from
a1b0e2c to
c1f3906
Compare
| payments[0].transaction.transaction_time = format( | ||
| getTime( | ||
| parse( | ||
| payments[0].transaction.transaction_date + |
There was a problem hiding this comment.
Can we use template literals `${}` here?
Also, an explanation for the 20 minutes thing? Is it just for the one test?
| parse(payments[0].transaction.transaction_time, 'HH:mm:ss', new Date()), | ||
| parse(deposits[0].transaction_time, 'HH:mm:ss', new Date()) | ||
| ) | ||
| ).toBeGreaterThanOrEqual(20); |
There was a problem hiding this comment.
What exactly are we testing here? Just that the 20 minute addition worked? It feels like we're just testing date-fns differenceInMinutes
| it('should match payments to deposits according to first round of heuristics', () => { | ||
| const spy = jest.spyOn(service, 'matchPosPaymentToPosDeposits'); | ||
| const roundOneTimeDiff = 5; | ||
| const timeDiff = roundOneTimeDiff; |
There was a problem hiding this comment.
Point of roundOneTimeDiff?
| timeDiff | ||
| ); | ||
| matches = [...matchedRoundOne]; | ||
| expect(spy).toBeCalled(); |
There was a problem hiding this comment.
Is there a scenario that this test fails?
| timeDiff | ||
| ); | ||
| matches.push(...matchedRoundTwo); | ||
| expect(spy).toBeCalled(); |
There was a problem hiding this comment.
I'm still wondering if there's a scenario in which this fails. And if so, how this is different from the other matchedRoundOne test, since it's the same function. I might need to read more on how the heuristics are working
| expect(itm.deposit.status === MatchStatus.MATCH); | ||
| expect(itm.payment.status === MatchStatus.MATCH); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Do we know exactly how many matches we have, and how many non-matches we have? Is there an argument for adding that as a test too?
| import { locations } from '../../mocks/const/locations'; | ||
| import { MockData } from '../../mocks/mocks'; | ||
|
|
||
| export const setSomePaymentsToTwentyMinutesLater = ( |
There was a problem hiding this comment.
This is also "resetting" the payments right (as in setting the status to pending)?
I'd suggest just passing an array into this function and calling it something like resetPaymentsDataset or something, and doing the 20 minute thing for each even index within the function rather than passing in an index. With some comments to explain why.
There was a problem hiding this comment.
The payments are reset between tests using:
(this is a recent change :) )
afterEach(() => {
jest.clearAllMocks();
});
Function has been updated to receive an array, and status updates are removed (it was redundant since the mock data generates the status as PENDING and each test will generate new data)
:)
apps/backend/invoke.sh
Outdated
| @@ -0,0 +1,21 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Probably should be in a separate pr
There was a problem hiding this comment.
oops wasn't planning to commit this one
| } | ||
| : { | ||
| ...itm, | ||
| timestamp: itm.timestamp |
There was a problem hiding this comment.
Isn't itm.timestamp covered within ...itm?
2 participants
CCFPCM-0419
Objective: