Closed zklgame closed 11 months ago
Attention: 6 lines
in your changes are missing coverage. Please review.
Comparison is base (
c21df66
) 54.67% compared to head (747d833
) 58.11%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Lgtm. It would be better to rebase to main branch and add some new test case for the completeExecute persistence api. The latest commit in main branch has made it simpler to add new persistence test case (see my msg in WeChat )
Added two more tests based on my best understanding. Though I think each test needs a long setup. Maybe we need to further refactor the testing codes to make a common setup.
Lgtm. It would be better to rebase to main branch and add some new test case for the completeExecute persistence api. The latest commit in main branch has made it simpler to add new persistence test case (see my msg in WeChat )
Added two more tests based on my best understanding. Though I think each test needs a long setup. Maybe we need to further refactor the testing codes to make a common setup.
I think the way “testMain” works is that it will reuse the same setup. You can see that it will only create test database once
Lgtm. It would be better to rebase to main branch and add some new test case for the completeExecute persistence api. The latest commit in main branch has made it simpler to add new persistence test case (see my msg in WeChat )
Added two more tests based on my best understanding. Though I think each test needs a long setup. Maybe we need to further refactor the testing codes to make a common setup.
I think the way “testMain” works is that it will reuse the same setup. You can see that it will only create test database once
The setup I am talking about is start process
and completeExecute to run other states
. Now for each test I need to do the same operations. So I think we can put the start process
and start other states
operations in a common shared testing logic?
The integ test failed after merging due to https://github.com/xdblab/xdb-golang-sdk/pull/17.
Why make this pull request?
Support GracefulComplete and ForceFail StateDecision.
What has changed
wait_to_complete
field to tablexdb_sys_process_executions
SELECT FOR UPDATE
lock toselectProcessExecutionForUpdateQuery
How to test this pull request?
https://github.com/xdblab/xdb-golang-sdk/pull/15