Skip to content

Flaky tests fix: Migration shim can apply stashed v1 ops to v1 state and v2 ops to v2 state #26148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MarioJGMsoft merged 8 commits into from
Jan 8, 2026
Merged

Flaky tests fix: Migration shim can apply stashed v1 ops to v1 state and v2 ops to v2 state #26148

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
91d3730
fix: removed unnecessary provider.ensureSynchronized()
MarioJGMsoft Jan 7, 2026
edc27a7
Merge remote-tracking branch 'upstream/main' into marioja/flakyTestFix
MarioJGMsoft Jan 7, 2026
5a93227
Merge branch 'main' into marioja/flakyTestFix
MarioJGMsoft Jan 8, 2026
4be93ad
fix: changed function deletion --> comment it out with comment for co…
MarioJGMsoft Jan 8, 2026
4749279
Merge branch 'marioja/flakyTestFix' of https://github.com/MarioJGMsof…
MarioJGMsoft Jan 8, 2026
d248522
fix: removed only from test
MarioJGMsoft Jan 8, 2026
56a299b
Merge branch 'main' into marioja/flakyTestFix
MarioJGMsoft Jan 8, 2026
9b71fb3
Merge branch 'main' into marioja/flakyTestFix
MarioJGMsoft Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/test/test-end-to-end-tests/src/test/migration-shim/reconnect.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,8 @@ describeCompat("Stamped v2 ops", "NoCompat", (getTestObjectProvider, apis) => {

const loader = provider.createLoader([[provider.defaultCodeDetails, runtimeFactory2]]);
const container2 = await loader.resolve({ url }, pendingState);
await provider.ensureSynchronized();
Copy link
Member

@markfields markfields Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, I think it's possible that container2 would be missing state from container1 that predates the local pending state...? But maybe that's not possible, I think it only applies pending state once all ops up to the refSeq are processed. So... maybe it is ok.

What exactly were the "timing issues" - timeouts? I'm unsure how else removing an await could fix the flakiness. And I also feel like if this were timing out we should open a bug to investigate root cause there. ensureSynchronized hanging usually indicates a bug (e.g. around dirty/saved state tracking)

Copy link
Contributor Author

@MarioJGMsoft MarioJGMsoft Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issues were that the container2 wasn't being saved properly and it was left hanging, causing the test to timeout when it was ran against FRS

// The following line causes a timeout when the test is ran against FRS.
// await provider.ensureSynchronized();
const testObj2 = (await container2.getEntryPoint()) as TestDataObject;
const shim2 = testObj2.getTree<MigrationShim>();
const legacyTree2 = shim2.currentTree as LegacySharedTree;
Expand Down Expand Up @@ -333,7 +334,8 @@ describeCompat("Stamped v2 ops", "NoCompat", (getTestObjectProvider, apis) => {

const loader = provider.createLoader([[provider.defaultCodeDetails, runtimeFactory2]]);
const container2 = await loader.resolve({ url }, pendingState);
await provider.ensureSynchronized();
// The following line causes a timeout when the test is ran against FRS.
// await provider.ensureSynchronized();
const testObj2 = (await container2.getEntryPoint()) as TestDataObject;
const shim2 = testObj2.getTree<MigrationShim>();
const newTree2 = shim2.currentTree as ITree;
Expand Down
Loading