fix: Avoid trigger last execution "unknown" while the connector is still running #1515
Conversation
doubleface
force-pushed
the
feat/watchJobAfterAccountCreate
branch
from
18a1d99 to
7fcdac1
Compare
| @@ -66,6 +66,11 @@ export class KonnectorJobWatcher { | |||
| this.emit('error', new KonnectorJobError(error)) | |||
| } | |||
|
|
|||
| handleLoginSuccess() { | |||
There was a problem hiding this comment.
nit: don't we test that part?
There was a problem hiding this comment.
This would need to be an integration test. I think there is no point to add a unit test here.
| }) | ||
|
|
||
| afterAll(() => { | ||
| jest.restoreAllMocks() |
There was a problem hiding this comment.
what is the point to use a clearAllMocks before restoreAllMocks:
https://jestjs.io/docs/mock-function-api#mockfnmockrestore
I read here that mockRestore does everything that mockReset does
And mockReset does everything that mockClear deos
There was a problem hiding this comment.
nit: i recommend to remove clearAllMocks
There was a problem hiding this comment.
I prefer to remove restoreAllMocks in this case. ClearAllMocks will erase the history of calls between test which is what I want in the first place.
And removing clearAllMocks makes the tests fail
There was a problem hiding this comment.
ok, then.
ps: I need to understand one day why mocks are not reset by default. 🤯
| }) | ||
|
|
||
| afterAll(() => { | ||
| jest.restoreAllMocks() |
There was a problem hiding this comment.
what is the point to use a clearAllMocks before restoreAllMocks:
https://jestjs.io/docs/mock-function-api#mockfnmockrestore
I read here that mockRestore does everything that mockReset does
And mockReset does everything that mockClear deos
There was a problem hiding this comment.
nit: i recommend to remove clearAllMocks
This is a problem especially visible with client side connectors but you can also reproduce it with node connecteur :
When you display the harvest dialog for a trigger which is still running and which runs for the first time, the trigger is marked as not running and the last execution date is "unknown"
For node connecteur, when the LOGIN_SUCCESS event is triggered, we display a popup which explains that authentication is a success and that the user can do someting else, then this bug is not too much pain
But for client side connectors, on LOGIN_SUCCESS, the harvest is displayed and the connector is displayed as not running, which is wrong and suggests an error :
Now, when a still running trigger is displayed, harvest will mark it a running and watch the corresponding job to get the result of the job and display it