chore: enable anchoring on recon mode #3185
Conversation
|
I'm going to hold off on reviewing this until CI is green, looks like there are still some remaining issues right now |
| @@ -152,7 +152,7 @@ test('Dont process the same entry multiple times concurrently', async () => { | |||
| async function* overEntries(): AsyncGenerator<number> { | |||
| do { | |||
| // Need this sleep or else the Node runtime might never switch back to the "process" function. | |||
| await CommonTestUtils.delay(1) | |||
| await CommonTestUtils.delay(5) | |||
There was a problem hiding this comment.
@ukstv This is how I solved it
There was a problem hiding this comment.
what's the issue you were seeing? This change really shouldn't be necessary. This sleep isn't actually making the test wait for anything to happen, it's just so that the Node event loop has an interrupt point so other jobs can get scheduled, otherwise the generator can just emit events forever without the task to process those events ever running. But increasing the duration of the sleep should have no affect on the rest of the system or the test, unless I really misunderstood something when I wrote this test.
There was a problem hiding this comment.
I'm assuming this test failure was the issue: https://app.circleci.com/pipelines/github/ceramicnetwork/js-ceramic/13094/workflows/f7e8914c-f718-40e3-bfd6-4ee07174026c/jobs/18766
Put together a PR that I believe should fix the race condition in the test: #3192
There was a problem hiding this comment.
yes that was the issue, thanks for the PR
There was a problem hiding this comment.
Can this change be reverted now that #3192 is in?
…-of-the-ceramic_recon_mode
…-of-the-ceramic_recon_mode
JulissaDantes
force-pushed
the
feature/ws2-3129-enable-anchoring-as-part-of-the-ceramic_recon_mode
branch
from
1c66918 to
f8fa7e9
Compare
|
I did not have to make changes to the code other than enabling the anchor when using recon, but I did have to adjust the test setup. A common modification was to add some delay, which is why you will see it in some tests. Ready to be reviewed by @stbrody and @nathanielc. |
| @@ -152,7 +152,7 @@ test('Dont process the same entry multiple times concurrently', async () => { | |||
| async function* overEntries(): AsyncGenerator<number> { | |||
| do { | |||
| // Need this sleep or else the Node runtime might never switch back to the "process" function. | |||
| await CommonTestUtils.delay(1) | |||
| await CommonTestUtils.delay(5) | |||
There was a problem hiding this comment.
Can this change be reverted now that #3192 is in?
packages/stream-tests/src/__tests__/model-instance-document.test.ts
Outdated
Show resolved
Hide resolved
packages/stream-tests/src/__tests__/model-instance-document.test.ts
Outdated
Show resolved
Hide resolved
packages/stream-tests/src/__tests__/model-instance-document.test.ts
Outdated
Show resolved
Hide resolved
packages/stream-tests/src/__tests__/model-instance-document.test.ts
Outdated
Show resolved
Hide resolved
| @@ -152,7 +152,7 @@ test('Dont process the same entry multiple times concurrently', async () => { | |||
| async function* overEntries(): AsyncGenerator<number> { | |||
| do { | |||
| // Need this sleep or else the Node runtime might never switch back to the "process" function. | |||
| await CommonTestUtils.delay(1) | |||
| await CommonTestUtils.delay(5) | |||
JulissaDantes
deleted the
feature/ws2-3129-enable-anchoring-as-part-of-the-ceramic_recon_mode
branch
Description