(GC) Update a GC end-to-end test to use sinon timers instead of real ones #23516
Conversation
github-actions
bot
added
area: tests
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts:685
- Replace 'tombstone ready' with 'sweep ready' for consistency.
// Summarize. The tombstone ready data store should get realized because it has a trailing op.
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts
Outdated
Show resolved
Hide resolved
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts
Outdated
Show resolved
Hide resolved
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts
Show resolved
Hide resolved
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts
Show resolved
Hide resolved
packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts
Outdated
Show resolved
Hide resolved
agarwal-navin
force-pushed
the
sinonTimerInGCTest
branch
from
bbae5bb to
1334d8b
Compare
|
|
||
| // Send an op to update the timestamp that the summarizer client uses for GC to a current one. | ||
| defaultDataObject._root.set("update", "timestamp"); | ||
| await provider.ensureSynchronized(); | ||
|
|
||
| // Close the container as it would be closed by session expiry before sweep ready ever occurs. |
There was a problem hiding this comment.
Keep this comment by the container.close where you moved it?
| summarizingContainer1.close(); | ||
|
|
||
| clock.tick(sweepTimeoutMs + 10); |
There was a problem hiding this comment.
Why'd you decide to move this?
There was a problem hiding this comment.
If it's done after loading the new summarize, its session will expire when the clock moves past sweep timeout.
|
|
||
| // Send a trailing op and close the summarizer. |
There was a problem hiding this comment.
Restore this? It's nice to highlight where the trailing op is, since that's a key part of the test
There was a problem hiding this comment.
I changed this code a bit and added another comment in place of this.
| const container2 = await provider.loadTestContainer(testContainerConfig); | ||
| await provider.ensureSynchronized(); | ||
| // Close the first container to simulate scenario where it closes before its session expires. | ||
| container.close(); |
There was a problem hiding this comment.
Does this prevent the "unexpected delete" log or whatever, about the local thing getting deleted?
There was a problem hiding this comment.
Yes, it does.
| @@ -125,7 +125,7 @@ function validateDataStoreStateInSummary( | |||
|
|
|||
| const tombstoneTimeoutMs = 200; | |||
| const sweepGracePeriodMs = 0; // Skip Tombstone, these tests focus on Sweep | |||
| const sweepTimeoutMs = tombstoneTimeoutMs + sweepGracePeriodMs; | |||
| const sweepTimeoutMs = tombstoneTimeoutMs + sweepGracePeriodMs; // defaultSweepGracePeriodMs; | |||
There was a problem hiding this comment.
| const sweepTimeoutMs = tombstoneTimeoutMs + sweepGracePeriodMs; // defaultSweepGracePeriodMs; | |
| const sweepTimeoutMs = tombstoneTimeoutMs + sweepGracePeriodMs; |
agarwal-navin
changed the title
This PR updates one of the GC end-to-end test to use sinon fake timers instead of real timers. The tests in this file adds delays to wait for sweep timers to expire. Using fake timers these delays have been replaced with advancing the clock thereby reducing the runtime.
The run time for this test file went from 4s to 2s.
AB#11248