mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-21 22:55:16 +00:00
* test(web-shell): pin silent failure of background artifact refreshes (#7427) The toast-spam behavior reported in #7427 no longer exists on main — loadArtifacts carries no notice dispatch and the hook swallows background-refresh failures, keeping the last-good artifacts. What was missing is a regression pin: add one that fails a background refresh and asserts last-good artifacts survive, loading clears, no error surfaces, and the next refresh recovers. Mutation-verified: clearing artifacts in the catch turns it red. * test(web-shell): cover artifact refresh triggers * test(web-shell): tighten artifact refresh assertions * test(web-shell): settle artifact-refresh mocks via deferred awaits (#7427) The two regression tests added here flushed their mocked refreshes with a single microtask, so under full-suite parallel load the hook's refresh continuation intermittently missed the React commit and the last-good assertions saw artifacts === [] — a signature indistinguishable from a real #7427 regression. Model every mocked load as a deferred and resolve/reject + await it inside act, the shape the file's pre-existing tests already use (review round, 7 fragile flush sites). Also fold in two review pins while the tests are being rewritten: - R2-2: the superseded-failure test now rejects the stale load while the superseding load is still in flight and asserts loading stays true — the requestId guard in the finally cleanup becomes load-bearing (mutation-verified: dropping the guard fails the test). - add a waiting -> idle settling-trigger test so the prompt guard cannot be specialized to 'streaming' ('waiting' is a real prompt status). * test(web-shell): pin the version bookkeeping and owner-guard halves (#7427) - Non-monotonic artifactsVersion sequence (1->2->1 from a non-zero start) with exact loadArtifacts call counts: killing the previous-value bookkeeping silently skips the refresh that returns to a previously-seen version (stale artifacts panel, suite green) — measured mutant. - Owner-flip supersede variant: the provider flips isCurrent() the instant the session switches, before a re-render; an in-flight load resolving in that window passes the requestId half and only the !owner.isCurrent() half of the guard stops it from painting the previous session's artifacts — deleting that half left all prior tests green (measured). * test(web-shell): pin superseded artifact successes --------- Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
This commit is contained in:
易良
GitHub
parent
6970c76114
commit
8a34d3eee1
1 changed files with 295 additions and 3 deletions
|
|
@ -20,6 +20,7 @@ Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
|
|||
interface Deferred<T> {
|
||||
promise: Promise<T>;
|
||||
resolve: (value: T) => void;
|
||||
reject: (reason?: unknown) => void;
|
||||
}
|
||||
|
||||
const sdkMock = vi.hoisted(() => ({
|
||||
|
|
@ -53,11 +54,15 @@ let latestState: SessionArtifactsState | undefined;
|
|||
|
||||
function deferred<T>(): Deferred<T> {
|
||||
let resolve: ((value: T) => void) | undefined;
|
||||
const promise = new Promise<T>((done) => {
|
||||
let reject: ((reason?: unknown) => void) | undefined;
|
||||
const promise = new Promise<T>((done, fail) => {
|
||||
resolve = done;
|
||||
reject = fail;
|
||||
});
|
||||
if (!resolve) throw new Error('deferred promise did not initialize');
|
||||
return { promise, resolve };
|
||||
if (!resolve || !reject) {
|
||||
throw new Error('deferred promise did not initialize');
|
||||
}
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
function artifact(id: string): DaemonSessionArtifact {
|
||||
|
|
@ -215,4 +220,291 @@ describe('useSessionArtifacts', () => {
|
|||
'replacement',
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps automatic refresh failures silent and recovers on the next refresh (#7427)', async () => {
|
||||
// A transient `Failed to fetch` on an automatic refresh is noise the user
|
||||
// cannot act on. The panel keeps last-good artifacts when it has them,
|
||||
// clears `loading`, and exposes no error state.
|
||||
//
|
||||
// Every mocked load is a deferred settled explicitly inside `act` (and
|
||||
// awaited there), the same shape the pre-existing tests in this file
|
||||
// use: flushing with a bare microtask left the hook's refresh
|
||||
// continuation off the commit under full-suite parallel load, making
|
||||
// the last-good assertions intermittently see `artifacts === []`
|
||||
// (#7427 review).
|
||||
const load1 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load2 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load3 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load4 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load5 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(load1.promise)
|
||||
.mockReturnValueOnce(load2.promise)
|
||||
.mockReturnValueOnce(load3.promise)
|
||||
.mockReturnValueOnce(load4.promise)
|
||||
.mockReturnValueOnce(load5.promise);
|
||||
|
||||
await renderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Automatic refresh #1: initial mount fails before any last-good data exists.
|
||||
await act(async () => {
|
||||
load1.reject(new Error('Failed to fetch'));
|
||||
await load1.promise.catch(() => undefined);
|
||||
});
|
||||
expect(latestState?.loading).toBe(false);
|
||||
expect(latestState?.error).toBeNull();
|
||||
expect(latestState?.artifacts).toEqual([]);
|
||||
|
||||
// Automatic refresh #2: artifactsVersion recovers and establishes last-good.
|
||||
sdkMock.artifactsVersion = 1;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(2);
|
||||
await act(async () => {
|
||||
load2.resolve({ artifacts: [artifact('current-artifact')] });
|
||||
await load2.promise;
|
||||
});
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
expect(latestState?.loading).toBe(false);
|
||||
|
||||
// Automatic refresh #3: prompt settling back to idle fails transiently.
|
||||
sdkMock.promptStatus = 'streaming';
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(2);
|
||||
sdkMock.promptStatus = 'idle';
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(3);
|
||||
await act(async () => {
|
||||
load3.reject(new Error('Failed to fetch'));
|
||||
await load3.promise.catch(() => undefined);
|
||||
});
|
||||
expect(latestState?.loading).toBe(false);
|
||||
expect(latestState?.error).toBeNull();
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
|
||||
// Automatic refresh #4: artifactsVersion fails and still keeps last-good.
|
||||
sdkMock.artifactsVersion = 2;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(4);
|
||||
await act(async () => {
|
||||
load4.reject(new Error('Failed to fetch'));
|
||||
await load4.promise.catch(() => undefined);
|
||||
});
|
||||
expect(latestState?.loading).toBe(false);
|
||||
expect(latestState?.error).toBeNull();
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
|
||||
// Re-rendering the same version is not a new automatic refresh.
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(4);
|
||||
|
||||
// The panel is not wedged: the next automatic refresh recovers.
|
||||
sdkMock.artifactsVersion = 3;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(5);
|
||||
await act(async () => {
|
||||
load5.resolve({ artifacts: [artifact('recovered-artifact')] });
|
||||
await load5.promise;
|
||||
});
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'recovered-artifact',
|
||||
]);
|
||||
});
|
||||
|
||||
it('ignores loading cleanup from superseded artifact refresh failures', async () => {
|
||||
const initialLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const staleLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const finalLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(initialLoad.promise)
|
||||
.mockReturnValueOnce(staleLoad.promise)
|
||||
.mockReturnValueOnce(finalLoad.promise);
|
||||
|
||||
await renderHookHost();
|
||||
await act(async () => {
|
||||
initialLoad.resolve({ artifacts: [artifact('current-artifact')] });
|
||||
await initialLoad.promise;
|
||||
});
|
||||
|
||||
sdkMock.artifactsVersion = 1;
|
||||
await rerenderHookHost();
|
||||
expect(latestState?.loading).toBe(true);
|
||||
|
||||
sdkMock.artifactsVersion = 2;
|
||||
await rerenderHookHost();
|
||||
// The stale failure lands while the superseding load is still in
|
||||
// flight — the requestId guard must keep its cleanup from clearing
|
||||
// `loading` prematurely (R2-2, #7427 review).
|
||||
await act(async () => {
|
||||
staleLoad.reject(new Error('Failed to fetch'));
|
||||
await staleLoad.promise.catch(() => undefined);
|
||||
});
|
||||
expect(latestState?.loading).toBe(true);
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
|
||||
await act(async () => {
|
||||
finalLoad.resolve({ artifacts: [artifact('replacement')] });
|
||||
await finalLoad.promise;
|
||||
});
|
||||
expect(latestState?.loading).toBe(false);
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'replacement',
|
||||
]);
|
||||
});
|
||||
|
||||
it('ignores stale success from superseded artifact refreshes', async () => {
|
||||
const initialLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const staleLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const finalLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(initialLoad.promise)
|
||||
.mockReturnValueOnce(staleLoad.promise)
|
||||
.mockReturnValueOnce(finalLoad.promise);
|
||||
|
||||
await renderHookHost();
|
||||
await act(async () => {
|
||||
initialLoad.resolve({ artifacts: [artifact('current-artifact')] });
|
||||
await initialLoad.promise;
|
||||
});
|
||||
|
||||
sdkMock.artifactsVersion = 1;
|
||||
await rerenderHookHost();
|
||||
sdkMock.artifactsVersion = 2;
|
||||
await rerenderHookHost();
|
||||
|
||||
await act(async () => {
|
||||
staleLoad.resolve({ artifacts: [artifact('stale-artifact')] });
|
||||
await staleLoad.promise;
|
||||
});
|
||||
expect(latestState?.loading).toBe(true);
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
|
||||
await act(async () => {
|
||||
finalLoad.resolve({ artifacts: [artifact('replacement')] });
|
||||
await finalLoad.promise;
|
||||
});
|
||||
expect(latestState?.loading).toBe(false);
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'replacement',
|
||||
]);
|
||||
});
|
||||
|
||||
it('refreshes when the version returns to a previously-seen value (#7427)', async () => {
|
||||
// The version effect's previous-value bookkeeping must survive a
|
||||
// NON-MONOTONIC sequence: starting at a non-zero version, a return to a
|
||||
// previously-seen value is still a transition. A mutant that forgets to
|
||||
// record the seen version keeps comparing against the mount value and
|
||||
// silently skips the returning refresh (stale panel, suite green).
|
||||
const load1 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load2 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const load3 = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(load1.promise)
|
||||
.mockReturnValueOnce(load2.promise)
|
||||
.mockReturnValueOnce(load3.promise);
|
||||
sdkMock.artifactsVersion = 1;
|
||||
|
||||
await renderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(1);
|
||||
await act(async () => {
|
||||
load1.resolve({ artifacts: [artifact('v1')] });
|
||||
await load1.promise;
|
||||
});
|
||||
|
||||
sdkMock.artifactsVersion = 2;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(2);
|
||||
await act(async () => {
|
||||
load2.resolve({ artifacts: [artifact('v2')] });
|
||||
await load2.promise;
|
||||
});
|
||||
|
||||
// Back to a previously-seen version — must fire again.
|
||||
sdkMock.artifactsVersion = 1;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(3);
|
||||
await act(async () => {
|
||||
load3.resolve({ artifacts: [artifact('v1-again')] });
|
||||
await load3.promise;
|
||||
});
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual(['v1-again']);
|
||||
});
|
||||
|
||||
it('drops an in-flight load whose session owner flipped mid-flight (#7427)', async () => {
|
||||
// The success guard is `requestId stale OR owner stale`. The requestId
|
||||
// half is exercised by the supersede tests; the OWNER half needs its own
|
||||
// pin: the provider flips `isCurrent` the instant the session switches —
|
||||
// BEFORE any re-render — so an in-flight load resolving in that window
|
||||
// passes the requestId check and only the owner check stops it from
|
||||
// painting the previous session's artifacts.
|
||||
const initialLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const inFlightLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(initialLoad.promise)
|
||||
.mockReturnValueOnce(inFlightLoad.promise);
|
||||
|
||||
await renderHookHost();
|
||||
await act(async () => {
|
||||
initialLoad.resolve({ artifacts: [artifact('current-artifact')] });
|
||||
await initialLoad.promise;
|
||||
});
|
||||
|
||||
// Start refresh #2 via a version bump (owner captured at version 0)...
|
||||
sdkMock.artifactsVersion = 1;
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(2);
|
||||
|
||||
// ...then the session flips mid-flight, without a re-render landing yet.
|
||||
sdkMock.ownerVersion += 1;
|
||||
await act(async () => {
|
||||
inFlightLoad.resolve({ artifacts: [artifact('stale')] });
|
||||
await inFlightLoad.promise;
|
||||
// Restore before the act finishes so later assertions see a clean
|
||||
// owner; the guard already evaluated against the flipped value.
|
||||
sdkMock.ownerVersion = 0;
|
||||
});
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'current-artifact',
|
||||
]);
|
||||
});
|
||||
|
||||
it('refreshes when the prompt settles from waiting to idle (#7427)', async () => {
|
||||
// `'waiting'` is a real prompt status (queued prompt / observer text
|
||||
// deltas before any generation signal); the settling trigger must not
|
||||
// be specialized to `'streaming'` (#7427 review).
|
||||
const initialLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
const settleLoad = deferred<{ artifacts: DaemonSessionArtifact[] }>();
|
||||
sdkMock.actions.loadArtifacts
|
||||
.mockReturnValueOnce(initialLoad.promise)
|
||||
.mockReturnValueOnce(settleLoad.promise);
|
||||
sdkMock.promptStatus = 'waiting';
|
||||
|
||||
await renderHookHost();
|
||||
await act(async () => {
|
||||
initialLoad.resolve({ artifacts: [artifact('current-artifact')] });
|
||||
await initialLoad.promise;
|
||||
});
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(1);
|
||||
|
||||
sdkMock.promptStatus = 'idle';
|
||||
await rerenderHookHost();
|
||||
expect(sdkMock.actions.loadArtifacts).toHaveBeenCalledTimes(2);
|
||||
await act(async () => {
|
||||
settleLoad.resolve({ artifacts: [artifact('settled-artifact')] });
|
||||
await settleLoad.promise;
|
||||
});
|
||||
expect(latestState?.artifacts.map((item) => item.id)).toEqual([
|
||||
'settled-artifact',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue