alex/auto-undo-redo: fix fuzz tests

alex/no-batches
alex 2024-04-10 15:56:31 +01:00
rodzic 5277297285
commit 0b4fd98f90
8 zmienionych plików z 164 dodań i 56 usunięć

Wyświetl plik

@ -555,13 +555,14 @@ export class Editor extends EventEmitter<TLEventMap> {
])
}
},
afterDelete: (record) => {
afterDelete: (record, source) => {
// page was deleted, need to check whether it's the current page and select another one if so
if (this.getInstanceState()?.currentPageId === record.id) {
const backupPageId = this.getPages().find((p) => p.id !== record.id)?.id
if (backupPageId) {
this.store.put([{ ...this.getInstanceState(), currentPageId: backupPageId }])
} else {
} else if (source === 'user') {
// fall back to ensureStoreIsUsable:
this.store.ensureStoreIsUsable()
}
}
@ -573,7 +574,7 @@ export class Editor extends EventEmitter<TLEventMap> {
},
},
instance: {
afterChange: (prev, next) => {
afterChange: (prev, next, source) => {
// instance should never be updated to a page that no longer exists (this can
// happen when undoing a change that involves switching to a page that has since
// been deleted by another user)
@ -586,8 +587,8 @@ export class Editor extends EventEmitter<TLEventMap> {
...instance,
currentPageId: backupPageId,
}))
} else {
// if there are no pages, bail out to `ensureStoreIsUsable`
} else if (source === 'user') {
// fall back to ensureStoreIsUsable:
this.store.ensureStoreIsUsable()
}
}

Wyświetl plik

@ -180,6 +180,7 @@ export class HistoryManager<R extends UnknownRecord> {
}
this.store.applyDiff(diffToUndo)
this.store.ensureStoreIsUsable()
this.stacks.set({ undos, redos })
} finally {
this.state = previousState
@ -227,6 +228,7 @@ export class HistoryManager<R extends UnknownRecord> {
}
this.store.applyDiff(diffToRedo)
this.store.ensureStoreIsUsable()
this.stacks.set({ undos, redos })
} finally {
this.state = previousState

Wyświetl plik

@ -245,6 +245,11 @@ export class Store<R extends UnknownRecord = UnknownRecord, Props = unknown> {
// @internal (undocumented)
ensureStoreIsUsable(): void;
extractingChanges(fn: () => void): RecordsDiff<R>;
filterChangesByScope(change: RecordsDiff<R>, scope: RecordScope): {
added: { [K in IdOf<R>]: R; };
updated: { [K_1 in IdOf<R>]: [from: R, to: R]; };
removed: { [K in IdOf<R>]: R; };
} | null;
// (undocumented)
_flushHistory(): void;
get: <K extends IdOf<R>>(id: K) => RecFromId<K> | undefined;

Wyświetl plik

@ -3594,6 +3594,103 @@
"isAbstract": false,
"name": "extractingChanges"
},
{
"kind": "Method",
"canonicalReference": "@tldraw/store!Store#filterChangesByScope:member(1)",
"docComment": "/**\n * Filters out non-document changes from a diff. Returns null if there are no changes left.\n *\n * @param change - the records diff\n *\n * @returns \n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "filterChangesByScope(change: "
},
{
"kind": "Reference",
"text": "RecordsDiff",
"canonicalReference": "@tldraw/store!RecordsDiff:type"
},
{
"kind": "Content",
"text": "<R>"
},
{
"kind": "Content",
"text": ", scope: "
},
{
"kind": "Reference",
"text": "RecordScope",
"canonicalReference": "@tldraw/store!~RecordScope:type"
},
{
"kind": "Content",
"text": "): "
},
{
"kind": "Content",
"text": "{\n added: { [K in "
},
{
"kind": "Reference",
"text": "IdOf",
"canonicalReference": "@tldraw/store!IdOf:type"
},
{
"kind": "Content",
"text": "<R>]: R; };\n updated: { [K_1 in "
},
{
"kind": "Reference",
"text": "IdOf",
"canonicalReference": "@tldraw/store!IdOf:type"
},
{
"kind": "Content",
"text": "<R>]: [from: R, to: R]; };\n removed: { [K in "
},
{
"kind": "Reference",
"text": "IdOf",
"canonicalReference": "@tldraw/store!IdOf:type"
},
{
"kind": "Content",
"text": "<R>]: R; };\n } | null"
},
{
"kind": "Content",
"text": ";"
}
],
"isStatic": false,
"returnTypeTokenRange": {
"startIndex": 6,
"endIndex": 13
},
"releaseTag": "Public",
"isProtected": false,
"overloadIndex": 1,
"parameters": [
{
"parameterName": "change",
"parameterTypeTokenRange": {
"startIndex": 1,
"endIndex": 3
},
"isOptional": false
},
{
"parameterName": "scope",
"parameterTypeTokenRange": {
"startIndex": 4,
"endIndex": 5
},
"isOptional": false
}
],
"isOptional": false,
"isAbstract": false,
"name": "filterChangesByScope"
},
{
"kind": "Property",
"canonicalReference": "@tldraw/store!Store#get:member",

Wyświetl plik

@ -258,7 +258,7 @@ export class Store<R extends UnknownRecord = UnknownRecord, Props = unknown> {
* @param change - the records diff
* @returns
*/
private filterChangesByScope(change: RecordsDiff<R>, scope: RecordScope) {
filterChangesByScope(change: RecordsDiff<R>, scope: RecordScope) {
const result = {
added: filterEntries(change.added, (_, r) => this.scopedTypes[scope].has(r.typeName)),
updated: filterEntries(change.updated, (_, r) => this.scopedTypes[scope].has(r[1].typeName)),
@ -791,8 +791,10 @@ export class Store<R extends UnknownRecord = UnknownRecord, Props = unknown> {
/** @internal */
ensureStoreIsUsable() {
this._integrityChecker ??= this.schema.createIntegrityChecker(this)
this._integrityChecker?.()
this.atomic(() => {
this._integrityChecker ??= this.schema.createIntegrityChecker(this)
this._integrityChecker?.()
})
}
private _isPossiblyCorrupted = false

Wyświetl plik

@ -1,5 +1,5 @@
/* eslint-disable no-console */
import { Editor, RecordsDiff } from '@tldraw/editor'
import { HistoryManager, RecordsDiff } from '@tldraw/editor'
// eslint-disable-next-line import/no-extraneous-dependencies
import { DiffOptions, diff as jestDiff } from 'jest-diff'
import { inspect } from 'util'
@ -72,8 +72,8 @@ export function prettyPrintDiff(diff: RecordsDiff<any>, opts?: DiffOptions) {
return prettyDiff
}
export function logHistory(editor: Editor) {
const { undos, redos, pendingDiff } = editor.history.debug()
export function logHistory(history: HistoryManager<any>) {
const { undos, redos, pendingDiff } = history.debug()
const p = new Printer()
p.log('=== History ===')
p.indent()

Wyświetl plik

@ -272,7 +272,9 @@ export class TLSyncClient<R extends UnknownRecord, S extends Store<R> = Store<R>
this.lastServerClock = 0
}
// kill all presence state
this.store.remove(Object.keys(this.store.serialize('presence')) as any)
this.store.mergeRemoteChanges(() => {
this.store.remove(Object.keys(this.store.serialize('presence')) as any)
})
this.lastPushedPresenceState = null
this.isConnectedToRoom = false
this.pendingPushRequests = []
@ -336,12 +338,22 @@ export class TLSyncClient<R extends UnknownRecord, S extends Store<R> = Store<R>
// then apply the upstream changes
this.applyNetworkDiff({ ...wipeDiff, ...event.diff }, true)
this.isConnectedToRoom = true
// now re-apply the speculative changes creating a new push request with the
// appropriate diff
const speculativeChanges = this.store.filterChangesByScope(
this.store.extractingChanges(() => {
this.store.applyDiff(stashedChanges)
}),
'document'
)
if (speculativeChanges) this.push(speculativeChanges)
})
// now re-apply the speculative changes as a 'user' to trigger
// creating a new push request with the appropriate diff
this.isConnectedToRoom = true
this.store.applyDiff(stashedChanges)
// this.isConnectedToRoom = true
// this.store.applyDiff(stashedChanges, false)
this.store.ensureStoreIsUsable()
// TODO: reinstate isNew

Wyświetl plik

@ -1,4 +1,3 @@
import isEqual from 'lodash.isequal'
import { nanoid } from 'nanoid'
import {
Editor,
@ -17,6 +16,9 @@ import { RandomSource } from './RandomSource'
import { TestServer } from './TestServer'
import { TestSocketPair } from './TestSocketPair'
// eslint-disable-next-line import/no-internal-modules
import { prettyPrintDiff } from '../../../tldraw/src/test/testutils/pretty'
jest.mock('@tldraw/editor/src/lib/editor/managers/TickManager.ts', () => {
return {
TickManager: class {
@ -75,8 +77,8 @@ class FuzzTestInstance extends RandomSource {
) {
super(seed)
this.store = createTLStore({ schema })
this.id = nanoid()
this.store = createTLStore({ schema, id: this.id })
this.socketPair = new TestSocketPair(this.id, server)
this.client = new TLSyncClient<TLRecord>({
store: this.store,
@ -109,9 +111,7 @@ class FuzzTestInstance extends RandomSource {
function assertPeerStoreIsUsable(peer: FuzzTestInstance) {
const diffToEnsureUsable = peer.store.extractingChanges(() => peer.store.ensureStoreIsUsable())
if (!isRecordsDiffEmpty(diffToEnsureUsable)) {
throw new Error(
`store of ${peer.id} was not usable. diff to ensure usable: ${JSON.stringify(diffToEnsureUsable, null, 2)}`
)
throw new Error(`store of ${peer.id} was not usable\n${prettyPrintDiff(diffToEnsureUsable)}`)
}
}
@ -221,6 +221,7 @@ function runTest(seed: number) {
if (!peer.socketPair.isConnected && peer.randomInt(2) === 0) {
peer.socketPair.connect()
allOk('final connect')
assertPeerStoreIsUsable(peer)
}
}
}
@ -234,6 +235,7 @@ function runTest(seed: number) {
allOk('final flushServer')
peer.socketPair.flushClientSentEvents()
allOk('final flushClient')
assertPeerStoreIsUsable(peer)
}
}
}
@ -243,29 +245,19 @@ function runTest(seed: number) {
assertPeerStoreIsUsable(peer)
}
const equalityResults = []
for (let i = 0; i < peers.length; i++) {
const row = []
for (let j = 0; j < peers.length; j++) {
row.push(
isEqual(
peers[i].editor?.store.serialize('document'),
peers[j].editor?.store.serialize('document')
)
)
// all stores should be the same
for (let i = 1; i < peers.length; i++) {
const expected = peers[i - 1]
const actual = peers[i]
try {
expect(actual.store.serialize('document')).toEqual(expected.store.serialize('document'))
} catch (e: any) {
throw new Error(`received = ${actual.id}, expected = ${expected.id}\n${e.message}`)
}
equalityResults.push(row)
}
const [first, ...rest] = peers.map((peer) => peer.editor?.store.serialize('document'))
// writeFileSync(`./test-results.${seed}.json`, JSON.stringify(ops, null, '\t'))
expect(first).toEqual(rest[0])
// all snapshots should be the same
expect(rest.every((other) => isEqual(other, first))).toBe(true)
totalNumPages += Object.values(first!).filter((v) => v.typeName === 'page').length
totalNumShapes += Object.values(first!).filter((v) => v.typeName === 'shape').length
totalNumPages += peers[0].store.query.ids('page').get().size
totalNumShapes += peers[0].store.query.ids('shape').get().size
} catch (e) {
console.error('seed', seed)
console.error(
@ -285,28 +277,25 @@ const NUM_TESTS = 50
const NUM_OPS_PER_TEST = 100
const MAX_PEERS = 4
// test.only('seed 8343632005032947', () => {
// runTest(8343632005032947)
// })
test('seed 8360926944486245 - undo/redo page integrity regression', () => {
runTest(8360926944486245)
})
test('seed 3467175630814895 - undo/redo page integrity regression', () => {
runTest(3467175630814895)
})
test('fuzzzzz', () => {
for (let i = 0; i < NUM_TESTS; i++) {
const seed = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER)
try {
runTest(seed)
} catch (e) {
console.error('seed', seed)
throw e
}
}
test('seed 6820615056006575 - undo/redo page integrity regression', () => {
runTest(6820615056006575)
})
test('seed 5279266392988747 - undo/redo page integrity regression', () => {
runTest(5279266392988747)
})
for (let i = 0; i < NUM_TESTS; i++) {
const seed = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER)
test(`seed ${seed}`, () => {
runTest(seed)
})
}
test('totalNumPages', () => {
expect(totalNumPages).not.toBe(0)