You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/09/12 09:10:49 UTC
[PR] test(desktop): reduce Electron coverage from 41 to 34 tests [maka]
Astro-Han opened a new pull request, #5228:
URL: https://github.com/apache/maka/pull/5228
## Summary
Desktop's Electron tier still started separate apps for fixture CSS and renderer-only settings/workbar behavior. This reduces the live-main budget from **41 tests in 20 files to 34 in 20 files**, removing seven fixture launches and one full Host history traversal.
- Move fixture motion and Chromium Turn render-skipping checks into `TranscriptRenderCost`, reusing the existing shell and a 32-Turn slice of the ManyTurns data. Keep the original 120-Turn catalog scene. Restrict visibility events to the Turn itself: bubbled events from nested blocks must not satisfy the containment assertion.
- Move loading-surface Escape into a small `SettingsOverlay` composition boundary used by production, and remove the `settings.chunk` preload latch. Move settings reopen continuity into the existing SettingsSurface story harness, retaining intermediate missing-control/loading-alert detection and the same renderer snapshot cache across unmounts.
- Exercise the composer usage entry in the existing per-Session workbar/reload test. Exercise settings workbar hide/restore in the existing pending-rename test, which now also reads the renamed record back from the Host. These retain real AppShell wiring without adding a second shell host.
- Combine mounted-range bounds and quiet-frame reader displacement in one real Host paging traversal; retain progress to the oldest Turn, carried-boundary evidence, return to latest, and the tail range bound. Preserve the original post-page sampling and existing thresholds/timeouts.
Refs #4761. Based on main `3cfcb09fd`. No changes are copied from #5192 (`b55755651`), and its worktree, branch, and PR are untouched. Both PRs touch `transcript-scroll-cost.spec.ts` and the budget; resolution must preserve #5192's new geometry protections. This PR retains main's quiet-frame displacement scope and does not claim #5192's input-period geometry coverage.
## Verification
- Affected Electron suite: 8 tests passed in 1.1 minutes, zero retries.
- Built Storybook; targeted play smoke for ManyTurns, TranscriptRenderCost, SettingsLoadingEscape, GeneralReopenKeepsReadyControls, and GeneralCachedRevalidation. The three added behavioral stories also pass at 4x CPU throttle with the existing 15-second smoke timeout.
- Mutation checks reject removal of the Escape listener, loss of the settings snapshot cache, removal of Turn containment, and restoration of fixture `transition: all 0.01ms`. All mutations reverted.
- Desktop typecheck, renderer architecture check (103 checker tests), E2E budget check (5 checker tests), settings resource/cache/authority unit tests (20), changed-file lint, and `git diff --check`.
- The unmodified main paging tests passed in 33.6 seconds combined; the merged traversal passed in 21.4 seconds in a local comparison. This is one local observation, not a benchmark or CI estimate.
Full Electron and full Storybook catalogs were not run. No retries or timeouts were increased. No product scroll implementation or CSS changed.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex implemented the test consolidation, renderer overlay extraction, and local validation.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [ ] Yes — described under Summary above
- [x] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(desktop): reduce Electron coverage from 41 to 34 tests [maka]
Posted by "zhiiw (via GitHub)" <gi...@apache.org>.
zhiiw commented on PR #5228:
URL: https://github.com/apache/maka/pull/5228#issuecomment-5648001675
Follow-up at exact head `2befb710` (verified unchanged; `test` completed/success on this head).
The [P1] above is closed: `TranscriptRenderCost` now exercises two distant scroll positions instead of forty incremental paints (same motion/skipping contracts, bounded work), and the hosted Storybook smoke passes on this head. The merge-conflict resolution in `preload.ts` keeps #5217's new `sessions.observe` latch while keeping the migrated `settings.chunk` latch removed — verified in the merged file.
My earlier content findings stand as written. Remaining boundary unchanged: I did not run the Electron or full Storybook suites locally.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(desktop): reduce Electron coverage from 41 to 34 tests [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #5228:
URL: https://github.com/apache/maka/pull/5228#discussion_r3995930266
##########
apps/desktop/stories/app-shell.stories.tsx:
##########
@@ -942,24 +944,112 @@ export const SafeResumeAfterRestart: Story = {
// Real path: a long session with 120 turns — past the transcript virtualizer's
// window, so it must stay correct and quiet where a handful of seeded turns
// would never trip the virtualization path.
+const manyTurnMessages = Array.from({ length: 120 }, (_, index) => {
+ const turnId = `turn-m-${index}`;
+ const minutesAgo = (120 - index) * 3;
+ return [
+ user(`msg-m-u-${index}`, turnId, minutesAgo, `第 ${index + 1} 轮：这个模块的边界条件该怎么覆盖？`),
+ assistant(`msg-m-a-${index}`, turnId, minutesAgo - 1, `第 ${index + 1} 轮回答：先列输入域，再对空、超长、并发三类分别加断言。`),
+ ];
+}).flat();
+
export const ManyTurns: Story = {
render: () => (
<ComposedShell
session={{ lastMessageAt: NOW - 60_000 }}
chat={{
- messages: Array.from({ length: 120 }, (_, index) => {
- const turnId = `turn-m-${index}`;
- const minutesAgo = (120 - index) * 3;
- return [
- user(`msg-m-u-${index}`, turnId, minutesAgo, `第 ${index + 1} 轮：这个模块的边界条件该怎么覆盖？`),
- assistant(`msg-m-a-${index}`, turnId, minutesAgo - 1, `第 ${index + 1} 轮回答：先列输入域，再对空、超长、并发三类分别加断言。`),
- ];
- }).flat(),
+ messages: manyTurnMessages,
}}
/>
),
};
+// The same transcript and fixture CSS, with enough resident Turns for native
+// render skipping but no need to mount the 120-Turn catalog demonstration.
+export const TranscriptRenderCost: Story = {
+ render: () => <ComposedShell frameHeight={700} chat={{ messages: manyTurnMessages.slice(-64) }} />,
+ play: async ({ canvasElement }) => {
+ const scroller = canvasElement.querySelector<HTMLElement>('[data-chat-scroll-container="true"]');
+ if (!scroller) throw new Error('Transcript scrollport is missing');
+ const frame = () => new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
+ await frame();
+ await frame();
+ let transitions = 0;
+ let animations = 0;
+ const skipped = new Set<Element>();
+ const turns = [...canvasElement.querySelectorAll<HTMLElement>('.maka-transcript-turn')];
+ expect(turns.length).toBeGreaterThan(0);
+ for (const pseudo of [null, '::before', '::after']) {
+ const style = getComputedStyle(turns[0], pseudo);
+ expect(style.transitionProperty).toBe('none');
+ expect(style.animationName).toBe('none');
+ }
+ const visibility = (event: Event) => {
+ if (event.target !== event.currentTarget) return;
+ const turn = event.currentTarget as Element;
+ if ((event as Event & { skipped: boolean }).skipped) skipped.add(turn);
+ else skipped.delete(turn);
+ };
+ for (const turn of turns) turn.addEventListener('contentvisibilityautostatechange', visibility);
+ const transition = () => { transitions += 1; };
+ const animation = () => { animations += 1; };
+ canvasElement.addEventListener('transitionrun', transition, true);
+ canvasElement.addEventListener('animationstart', animation, true);
+ try {
+ // No Host paging or wheel routing is under test: move the real Chromium
+ // scrollport to exercise the fixture CSS and browser render skipping.
+ for (const direction of [-1, 1]) {
+ for (let tick = 0; tick < 20; tick += 1) {
+ scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: direction * 120, bubbles: true }));
+ scroller.scrollTop += direction * 120;
+ await frame();
+ }
+ }
+ await waitFor(() => expect(skipped.size).toBeGreaterThan(0));
Review Comment:
Addressed in 2befb710c0602195f65ab6bf5157868ba21ab9e4. The cited run 34686338729 tested the previous head, 93094cacb. I replaced forty incremental paints with two distant scroll positions while retaining the motion and Turn render-skipping assertions, without increasing timeouts or adding retries. Mutations that disable Turn skipping or restore transitions fail their respective assertions.
The hosted test job now passes on the current head: https://github.com/apache/maka/actions/runs/34687308456/job/103536434667 . Its log explicitly reports TranscriptRenderCost passing and the complete Storybook smoke passing (337 stories, 364 theme renders). This closes the reported hosted-smoke failure; it does not establish the suggested viewport mechanism as the cause.
Automated response: OpenAI Codex, acting for the PR author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(desktop): reduce Electron coverage from 41 to 34 tests [maka]
Posted by "zhiiw (via GitHub)" <gi...@apache.org>.
zhiiw commented on code in PR #5228:
URL: https://github.com/apache/maka/pull/5228#discussion_r3995885039
##########
apps/desktop/stories/app-shell.stories.tsx:
##########
@@ -942,24 +944,112 @@ export const SafeResumeAfterRestart: Story = {
// Real path: a long session with 120 turns — past the transcript virtualizer's
// window, so it must stay correct and quiet where a handful of seeded turns
// would never trip the virtualization path.
+const manyTurnMessages = Array.from({ length: 120 }, (_, index) => {
+ const turnId = `turn-m-${index}`;
+ const minutesAgo = (120 - index) * 3;
+ return [
+ user(`msg-m-u-${index}`, turnId, minutesAgo, `第 ${index + 1} 轮：这个模块的边界条件该怎么覆盖？`),
+ assistant(`msg-m-a-${index}`, turnId, minutesAgo - 1, `第 ${index + 1} 轮回答：先列输入域，再对空、超长、并发三类分别加断言。`),
+ ];
+}).flat();
+
export const ManyTurns: Story = {
render: () => (
<ComposedShell
session={{ lastMessageAt: NOW - 60_000 }}
chat={{
- messages: Array.from({ length: 120 }, (_, index) => {
- const turnId = `turn-m-${index}`;
- const minutesAgo = (120 - index) * 3;
- return [
- user(`msg-m-u-${index}`, turnId, minutesAgo, `第 ${index + 1} 轮：这个模块的边界条件该怎么覆盖？`),
- assistant(`msg-m-a-${index}`, turnId, minutesAgo - 1, `第 ${index + 1} 轮回答：先列输入域，再对空、超长、并发三类分别加断言。`),
- ];
- }).flat(),
+ messages: manyTurnMessages,
}}
/>
),
};
+// The same transcript and fixture CSS, with enough resident Turns for native
+// render skipping but no need to mount the 120-Turn catalog demonstration.
+export const TranscriptRenderCost: Story = {
+ render: () => <ComposedShell frameHeight={700} chat={{ messages: manyTurnMessages.slice(-64) }} />,
+ play: async ({ canvasElement }) => {
+ const scroller = canvasElement.querySelector<HTMLElement>('[data-chat-scroll-container="true"]');
+ if (!scroller) throw new Error('Transcript scrollport is missing');
+ const frame = () => new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
+ await frame();
+ await frame();
+ let transitions = 0;
+ let animations = 0;
+ const skipped = new Set<Element>();
+ const turns = [...canvasElement.querySelectorAll<HTMLElement>('.maka-transcript-turn')];
+ expect(turns.length).toBeGreaterThan(0);
+ for (const pseudo of [null, '::before', '::after']) {
+ const style = getComputedStyle(turns[0], pseudo);
+ expect(style.transitionProperty).toBe('none');
+ expect(style.animationName).toBe('none');
+ }
+ const visibility = (event: Event) => {
+ if (event.target !== event.currentTarget) return;
+ const turn = event.currentTarget as Element;
+ if ((event as Event & { skipped: boolean }).skipped) skipped.add(turn);
+ else skipped.delete(turn);
+ };
+ for (const turn of turns) turn.addEventListener('contentvisibilityautostatechange', visibility);
+ const transition = () => { transitions += 1; };
+ const animation = () => { animations += 1; };
+ canvasElement.addEventListener('transitionrun', transition, true);
+ canvasElement.addEventListener('animationstart', animation, true);
+ try {
+ // No Host paging or wheel routing is under test: move the real Chromium
+ // scrollport to exercise the fixture CSS and browser render skipping.
+ for (const direction of [-1, 1]) {
+ for (let tick = 0; tick < 20; tick += 1) {
+ scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: direction * 120, bubbles: true }));
+ scroller.scrollTop += direction * 120;
+ await frame();
+ }
+ }
+ await waitFor(() => expect(skipped.size).toBeGreaterThan(0));
Review Comment:
[P1] The hosted `test` job fails on this head in Storybook smoke: `[product-shell-official-appshell--transcript-render-cost (light/default)] story did not finish rendering: page.waitForFunction: Timeout 15000ms exceeded` (run 34686338729). The story passed locally per the PR, so the behavior differs between the author's machine and the hosted smoke runner. Plausible mechanism to check first: this `waitFor(skipped.size > 0)` assumes the browser actually skips offscreen Turns — if the smoke runner's viewport/layout keeps every mounted Turn relevant, `skipped` never grows and the story hangs until the harness timeout. Until the story finishes in the hosted smoke, the migrated containment/motion coverage it carries is unproven at the merge gate.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(desktop): reduce Electron coverage from 41 to 34 tests [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #5228:
URL: https://github.com/apache/maka/pull/5228
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org