Issue 579 fix unit testing #593
Conversation
|
|
||
| it('should create a /sell', async () => { | ||
| const client = server.getClient(token, { timeout: 5000 }); | ||
| const command = client.makeCommand('/sell 100 1 ves Pagomovil'); |
There was a problem hiding this comment.
I have a doubt about this test, if I change the commando to a wrong command, let's say client.makeCommand('/sello 100 1 ves Pagomovil'); the test still passes, what are we testing here? can we make the test fails if the command is wrong?
There was a problem hiding this comment.
I will add the fail tests, the reason is that the telegram api is mocked with a certain response, for the fails tests it's necessary to add the mocks for the failed tests
There was a problem hiding this comment.
The unit tests have a controlled environment, we need to add all the mocked data to makes the test run every possible scenario
There was a problem hiding this comment.
is it possible to add some mocked data on this PR? not necessary all data but something to start
There was a problem hiding this comment.
Yes, I'm working on that
| @@ -345,15 +340,13 @@ const validateInvoice = async (ctx: MainContext, lnInvoice: string) => { | |||
| const latestDate = new Date( | |||
| Date.now() + Number(process.env.INVOICE_EXPIRATION_WINDOW) | |||
| ); | |||
| if (!("MAIN_PAYMENT_AMT" in process.env)) throw Error("MIN_PAYMENT_AMT not found, please check .env file"); | |||
| if (!("MIN_PAYMENT_AMT" in process.env)) throw Error("MIN_PAYMENT_AMT not found, please check .env file"); | |||
There was a problem hiding this comment.
@danfercf1 this looks like an unrelated change that should go in a separate PR?
There was a problem hiding this comment.
Yes, you are right the problem is that the test related to this implementation will not work. I've reported that problem in this issue #594
Fixes #579