Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); test(desktop): merge same-fixture e2e assertions into coherent journeys by UncertaintyDeterminesYou4ndMe · Pull Request #2486 · apache/maka · GitHub
Skip to content

test(desktop): merge same-fixture e2e assertions into coherent journeys - #2486

Closed
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys
Closed

test(desktop): merge same-fixture e2e assertions into coherent journeys#2486
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:perf/2390-e2e-journeys

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #2390 (Electron E2E half; the Storybook matrix reduction is #2481 per the issue's separate-PR requirement).

Problem

Every e2e window fixture is function-scoped, so 69 tests paid 69 Electron launches — and most of them re-launched and re-seeded exactly the state their file-mates had just built. quote-companion.spec.ts alone launched 12 windows over one identical seed; new-messages-indicator.spec.ts re-ran its six-message transcript seed from scratch for its second test.

Change

Tests that share one fixture and compatible assertions now run as consecutive phases of one journey. 69 tests / 69 launches → 34 tests / 34 launches. Every merge is annotated in-file with the ordering constraint that made it safe:

  • Count pins run first (e.g. 总消息数 === 1 phases, the blocked-Skill phase that pins zero turns and zero sessions).
  • Destructive phases run last (rename in settings-projects, the don't-ask-again checkbox that suppresses close confirmations for the rest of a window).
  • Turn-settle waits guard every phase boundary that sends — an Enter during a streaming turn silently becomes steering, which two merges surfaced immediately.
filetestsfiletests
quote-companion12 → 4composer-skill-invocation7 → 3
settings6 → 2mcp5 → 2
composer-mention-token4 → 1send-message4 → 2
providers3 → 1settings-projects3 → 1
7 two-test files2 → 1 each

Also removes 11 window fixtures no spec references (dead seeding code left behind by earlier test deletions, fixtures.ts 592 → 427 lines).

Deliberately not merged, and why

  • storage-root-conflict: the cold-start race is the subject; sharing a window would erase the precondition.
  • permission-mode-surface: its two tests boot two different fixture scenarios.
  • The staged-Skills draft-restoration test keeps its own window — observed, not guessed: merged into a shared window, its final send serialized raw /skill: text instead of chips, because concurrent session activity triggers exactly the external-value-change editor rebuild the test guards. That failure mode is worth a launch.
  • quote-companion's batch-close: needs the confirmation dialog the numbered-tab journey suppresses.
  • Cross-file candidates (the invocableSkillsWindow family spans 5 files; sandboxBoundaryWindow spans 2) are left as a maintainer decision — merging across files moves regression ownership between files, which is an organizational call, not a mechanical one.

Distinct contracts preserved

All IPC, focus, scrolling, remount, persistence, lifecycle, and animation assertions are unchanged — phases are the original test bodies with their assertions intact (three assertions were strengthened in passing: hostile-Mermaid now pins settled-turn count 2, the markdown --markdown scan asserts exit 0 explicitly, ask-user-question now answers after the reload, proving the rehydrated prompt is the live parked turn).

Timing

Same machine, warm build; CI runs workers=1 where launch count dominates:

configbeforeafter
workers=4 (local)69 passed, 57.9s34 passed, 47.3 / 43.2 / 40.8s (3 rounds)
workers=1 (CI-like)69 passed, 192.4s34 passed, 147.2s (−23%)

Electron launches: 69 → 34. Typecheck and biome are clean.

cc @Astro-Han

Part of apache#2390 (Electron E2E half; the Storybook matrix is a separate PR
per the issue).
Every e2e window fixture is function-scoped, so 69 tests paid 69
Electron launches — with most tests re-launching and re-seeding the
exact state their file-mates had just built. Tests that share one
fixture and compatible assertions now run as consecutive phases of one
journey; every merge is annotated with the ordering constraint that
made it safe (count pins run first, destructive phases run last,
don't-ask-again phases end their window).
Merged: ask-user-question 2->1, attachment 2->1, bot-onboarding 2->1,
composer-mention-token 4->1, composer-skill-invocation 7->3,
keyboard-help 2->1, mcp 5->2, new-messages-indicator 2->1 (also stops
re-seeding the six-message transcript), providers 3->1,
quote-companion 12->4, send-message 4->2, session-workbar 2->1,
settings-projects 3->1, settings 6->2, skill-delete-scope 2->1.
Kept separate deliberately:
- storage-root-conflict (cold-start race is the subject),
- permission-mode-surface (two different fixture scenarios),
- the staged-Skills draft-restoration test (its contract is editor
rebuilds on external value changes; concurrent session activity in a
shared window perturbs exactly that — observed, not guessed),
- quote-companion's batch-close (needs the confirmation dialog the
numbered-tab journey suppresses via don't-ask-again).
Also removes 11 window fixtures that no spec references (dead seeding
code left behind by earlier test deletions): longTranscriptWindow,
shortFinalTurnWindow, overflowingRailWindow, sidebarLongSessionsWindow,
disclosureOutputWindow, staleSessionsWindow, gitReviewWindow,
gitReviewLargeWindow, artifactPaneWindow, localeSwitchWindow,
planRemindersWindow.
69 tests / 69 launches -> 34 tests / 34 launches. Local (4 workers):
57.9s -> 47.3/43.2/40.8s across three green rounds. CI runs a single
worker, where launch count dominates wall time.
@jackwener

Copy link
Copy Markdown
Member

Review decision

Problem is real: function-scoped fixtures re-launch Electron for identical seeds; launch count dominates CI wall time.

Approach is correct: merge same-fixture assertions into ordered phases (count pins first, destructive last, turn-settle between sends), keep must-isolate cases (storage-root-conflict, permission-mode dual scenarios, staged-Skills draft restore).

Landing via maintainer branch because the fork head conflicts with main after #2478 presentation cleanup. Rebase drops reintroduced presentation journeys (MCP layout matrix, providers header geometry, settings back-icon rail) while keeping product journeys and dead-fixture removal.

See the rebased PR for the final tree.

@jackwener

Copy link
Copy Markdown
Member

Landing via rebased #2489 (presentation pins already removed on main via #2478 are not reintroduced). Thanks @UncertaintyDeterminesYou4ndMe!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@UncertaintyDeterminesYou4ndMe@jackwener@bekk59