Skip to content

BA-766 Update & Refactor tests#96

Merged
DjellzeBllaca merged 6 commits intodevelopfrom
BA-766-test-refactor
Jul 29, 2025
Merged

BA-766 Update & Refactor tests#96
DjellzeBllaca merged 6 commits intodevelopfrom
BA-766-test-refactor

Conversation

@DjellzeBllaca
Copy link
Copy Markdown
Contributor

No description provided.

@vildanbina vildanbina self-requested a review July 28, 2025 07:57
@Buckaroo-Rene Buckaroo-Rene requested a review from ShuCh3n July 28, 2025 09:44
Comment thread tests/PaymentMethods/AfterPay.test.ts Outdated
expect(data.isSuccess()).toBeTruthy();
});
});
}); //Add 10s timeout as third param
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remove this comment? it doesn't seem useful

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

.specification(RequestTypes.Transaction)
.request()
.then((data) => {
console.log(data);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you delete this log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +7 to +9
beforeEach(() => {
method = buckarooClientTest.method('transfer');
});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you follow the same approach with the tests above? I hadn’t noticed it there. Let’s use a consistent flow for all of them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread tests/PaymentMethods/Billink.test.ts Outdated
Comment on lines +71 to +76
// const authResponse = await method.authorize(payload).request();
// expect(authResponse.isSuccess()).toBeTruthy();
// let key = authResponse.getTransactionKey();
// expect(key).toBeDefined();

// new Promise((resolve) => setTimeout(resolve, 6000));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you delete these comments if they’re not useful?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment thread tests/PaymentMethods/Billink.test.ts Outdated
amountDebit: payload.amountDebit,
articles: payload.articles,
...payload,
originalTransactionKey: '1234',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use real transaction keys (as you did in the other cases) to avoid confusion when reviewing these tests and their transaction key format?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +15 to +20
const getServiceParameter = (response: any, name: string): string => {
const param = (response.getServices()?.[0]?.parameters as { name: string; value: any }[] | undefined)?.find(
(p) => p.name === name
);
return String(param?.value ?? '');
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you could move this to the test helper file, you might need it elsewhere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it from this class since it is already defined in a helper class

});
.request();

console.log(response.getStatusCode());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you delete this log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread tests/PaymentMethods/Emandate.test.ts Outdated
Comment on lines +40 to +50
// No valid subscription found for service 'emandateb2b'.
// test('CancelMandate', async () => {
// const response = await method
// .setServiceCode('emandateb2b' as ServiceCode)
// .cancelMandate({
// mandateId: '1DC7F83B7B937864FB39966B0C08A7B86D8',
// purchaseId: '6383d3e86944a0',
// })
// .request();
// expect(response.isSuccess()).toBeTruthy();
// });
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm maybe we should enable this subscription "emandate" in Plaza (check with @Buckaroo-Rene)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread tests/PaymentMethods/In3Old.test.ts Outdated
});

describe('Testing capayable methods', () => {
// "Action Pay is no longer available for Capayable"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this means?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vildanbina
Seems like PayInInstallments and refund are the only actions supported by Capayable (In3Old).
Check the docs.

However, we still have the pay action defined in our implementation. Should I remove it?

Comment thread package.json Outdated
"axios": "^1.6.2",
"crypto-js": "^4.1.1",
"ip-address": "^8.1.0",
"lodash": "^4.17.21",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if lodash is used only for tests atm, can you move this to devDependencies for now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@vildanbina
Copy link
Copy Markdown
Collaborator

and finally, please run prettier to format the code

@DjellzeBllaca DjellzeBllaca requested a review from vildanbina July 29, 2025 12:14
@DjellzeBllaca DjellzeBllaca merged commit a786b45 into develop Jul 29, 2025
1 check passed
@DjellzeBllaca DjellzeBllaca deleted the BA-766-test-refactor branch March 26, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants