Review StoredProc Reaction #103
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ruokun-niu
requested review from
amansinghoriginal,
danielgerlag and
agentofreality
| console.log(`Executing the stored proc: ${query}`); | ||
|
|
||
| // Execute the query | ||
| knex.raw(query).then(() => { |
There was a problem hiding this comment.
you could make this function async and use await
| /** | ||
| * @param {Array} queryIds | ||
| */ | ||
| function storedprocReactionManifest(queryIds) { |
There was a problem hiding this comment.
this could be pure yaml in the directory of your specific scenario
e2e-tests/simple-scenario/my.test.js
Outdated
|
|
||
|
|
||
| test('Test StoredProc Reaction - AddedResultCommand', async () => { |
There was a problem hiding this comment.
Let's create a new scenario instead of extending the simple scenario.
There was a problem hiding this comment.
I investigated this a bit more and found out that jest actually does not allow two test scenarios to run sequentially. This sometimes caused a race condition in the postgres database and the e2e test to fail.
I think maybe it is better to move the setup of the postgres database outside of the individual test scenarios and include it as a part of the cluster setup process?
There was a problem hiding this comment.
It would be better if scenarios did not share things. We could use different DNS names for different PG instances. I think you can run tests serially with jest: https://jestjs.io/docs/cli#--runinband
There was a problem hiding this comment.
Gotcha so one postgres per scenario
There was a problem hiding this comment.
Working on it right now
ruokun-niu
changed the title
|
|
||
|
|
||
|
|
||
| CREATE TABLE "CommandResult" ( |
There was a problem hiding this comment.
Do we still need this?
There was a problem hiding this comment.
The storedprocedure adds an item to this table. This is used to verify that the storedproc reaction is working
There was a problem hiding this comment.
but this is on the wrong scenario?
There was a problem hiding this comment.
Yeah I am moving this yaml
| await dbClient.query(`INSERT INTO "Item" ("ItemId", "Name", "Category") VALUES (3, 'Drasi', '1')`); | ||
|
|
||
| // sleep 35 seconds | ||
| await new Promise(r => setTimeout(r, 15000)); |
There was a problem hiding this comment.
Is there a better way we can do this rather than sleeping 35 seconds?
Could we check every 1 second and timeout after 30 seconds?
Something like the .waitForChange on on the signalr fixture.
| await knex.raw(query); | ||
| console.log("The query was executed successfully"); | ||
| } catch (error) { | ||
| console.log(error); |
There was a problem hiding this comment.
If it fails, we probably want the exception to bubble up to the event handler, so that it will return a 500 error back to DAPR, so that the message will not be acknowledged off the queue.
Description
e2efolderType of change
Fixes: (https://dev.azure.com/azure-octo/Incubations/_boards/board/t/Drasi/User%20Stories?workitem=12558)