Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds background pre-pull tasks for Kueue manifests and executor images in the E2E boot action, introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Boot as e2e-boot Action
participant Net as Manifest Server
participant K3s as k3s/crictl
participant FS as Filesystem (/tmp)
Runner->>Boot: start e2e-boot
Boot->>Net: nohup download Kueue manifest -> /tmp/kueue-manifests.yaml
Boot->>K3s: nohup pull python:3.11-slim
Boot->>K3s: nohup pull busybox:1.36
Boot->>FS: write PIDs and redirect logs, record exit codes to /tmp/*.exit
Note right of FS: background tasks run independently
Runner->>Runner: continue workflow
Runner->>FS: e2e-ready tails logs and checks PIDs/exit files
alt all background tasks succeeded
Runner->>Runner: apply manifests / start services
else any task failed
Runner->>Runner: fail workflow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR optimizes CI execution time by reducing redundant uv syncing during Python checks and by overlapping slow E2E setup work (Kueue manifest download + image pulls) in the background.
Changes:
- Use
uv run --no-syncin unit tests and lint/typecheck workflows to avoid re-syncing afteruv sync. - Speed up/parallelize E2E environment preparation by moving Kueue manifest download + executor image pre-pulls into
e2e-bootand gating Kueue install via a newinstall-kueueinput. - Add step names and minor wait-loop tuning (sleep 2s → 1s) for faster readiness checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/stack-tests.yml | Uses --no-sync, names E2E steps, and disables Kueue install for frontend E2E shards |
| .github/workflows/ruff.yml | Uses uv run --no-sync for ruff |
| .github/workflows/mypy.yml | Uses uv run --no-sync for mypy |
| .github/workflows/grimp.yml | Uses uv run --no-sync for grimp script |
| .github/actions/e2e-boot/action.yml | Adds background prefetch of Kueue manifest and executor runtime images |
| .github/actions/e2e-ready/action.yml | Adds install-kueue input; waits for background prefetch and makes Kueue install conditional |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/e2e-ready/action.yml:
- Around line 43-46: The background wait using `tail --pid="$(cat
/tmp/kueue-download.pid)" -f /dev/null` must be bounded to avoid hung CI; wrap
that call with a timeout (e.g. `timeout 300s`) so the wait ends after a fixed
period, and surface a clearer failure if the timeout occurs instead of silently
continuing — update the snippet that reads `/tmp/kueue-download.pid`,
`/tmp/kueue-download.log`, and checks `/tmp/kueue-download.exit` to use
`timeout` around `tail`, preserve the `|| true` where appropriate, and add an
explicit check that treats a timed-out tail as a failure (including a
descriptive message) before evaluating the exit file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b65a731-96ce-4946-8f40-687f32c84bf1
📒 Files selected for processing (6)
.github/actions/e2e-boot/action.yml.github/actions/e2e-ready/action.yml.github/workflows/grimp.yml.github/workflows/mypy.yml.github/workflows/ruff.yml.github/workflows/stack-tests.yml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/actions/e2e-ready/action.yml">
<violation number="1" location=".github/actions/e2e-ready/action.yml:43">
P2: The `tail --pid=... -f /dev/null` call has no timeout. If the background kueue download process hangs, this step will block indefinitely until the job-level hard timeout is reached, wasting CI minutes. Wrap with `timeout 300` (or similar) to fail fast.</violation>
<violation number="2" location=".github/actions/e2e-ready/action.yml:84">
P2: Same unbounded `tail --pid` wait issue here — if the python image pull hangs, this blocks until the job hard timeout. Add a `timeout` wrapper to fail fast.</violation>
<violation number="3" location=".github/actions/e2e-ready/action.yml:88">
P2: Same unbounded `tail --pid` wait issue here — if the busybox image pull hangs, this blocks until the job hard timeout. Add a `timeout` wrapper to fail fast.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/package.json`:
- Around line 74-76: package.json currently lists "jsdom" as a devDependency but
vitest.config.ts is configured to use "happy-dom" and there are no active
imports of jsdom; remove the "jsdom" entry from devDependencies in package.json,
run your package manager to update the lockfile (npm/yarn/pnpm install) and
verify no code imports or requires reference "jsdom" (search the repo for
"jsdom" and confirm only a comment remains); update any CI or tooling configs if
they explicitly install jsdom.
In `@frontend/vitest.setup.ts`:
- Around line 92-98: Remove the HTMLSelectElement.prototype.querySelectorAll
override: delete the origQSA assignment and the entire function that replaces
querySelectorAll (the code referencing origQSA and returning
Array.from(this.options)). Keep only the existing
HTMLSelectElement.prototype.querySelector override used for ':checked' handling
(used for Svelte bind:value). Ensure no other code relies on the removed
override and that querySelectorAll behavior falls back to the native
implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0678bd6e-640b-4900-9324-4405a7276b5d
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/workflows/mypy.ymlfrontend/package.jsonfrontend/src/__tests__/test-utils.tsfrontend/src/components/Pagination.sveltefrontend/src/components/__tests__/Pagination.test.tsfrontend/src/components/admin/AutoRefreshControl.sveltefrontend/src/routes/admin/AdminExecutions.sveltefrontend/src/routes/admin/AdminSagas.sveltefrontend/src/routes/admin/AdminUsers.sveltefrontend/src/routes/admin/__tests__/AdminEvents.test.tsfrontend/src/routes/admin/__tests__/AdminExecutions.test.tsfrontend/src/routes/admin/__tests__/AdminSagas.test.tsfrontend/src/routes/admin/__tests__/AdminUsers.test.tsfrontend/vitest.config.tsfrontend/vitest.setup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/mypy.yml
There was a problem hiding this comment.
2 issues found across 28 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/lib/notifications/__tests__/stream.test.ts">
<violation number="1" location="frontend/src/lib/notifications/__tests__/stream.test.ts:8">
P2: `vi.mocked` does not mock the API function; without `vi.mock`/`vi.spyOn`, `mockImplementation` will be undefined and the test will fail or call the real API. Add an explicit mock or spy for `$lib/api` before using `mockImplementation`.</violation>
</file>
<file name="frontend/src/routes/__tests__/Login.test.ts">
<violation number="1" location="frontend/src/routes/__tests__/Login.test.ts:95">
P2: `getErrorMessage` is no longer mocked, so `vi.mocked(getErrorMessage).mockReturnValue(...)` will fail because it’s a real function. Restore a module mock or spy on the module function before using mockReturnValue.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/lib/notifications/__tests__/stream.test.ts (1)
54-62:⚠️ Potential issue | 🟡 MinorAdd required test hook hygiene:
vi.clearAllMocks()andcleanup().The file is missing the required
beforeEach/afterEachcleanup pattern, which can cause test state leakage.Suggested patch
+import { cleanup } from '@testing-library/svelte'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; @@ beforeEach(() => { + vi.clearAllMocks(); callback = vi.fn(); mockSseFn.mockReset(); notificationStream.disconnect(); }); afterEach(() => { + cleanup(); vi.unstubAllGlobals(); });Per coding guidelines: "Call
cleanup()in VitestafterEach,vi.clearAllMocks()inbeforeEach".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/notifications/__tests__/stream.test.ts` around lines 54 - 62, Add the missing test-hygiene calls: in the existing beforeEach (where callback, mockSseFn.mockReset and notificationStream.disconnect are already called) add vi.clearAllMocks() to reset mocks before each test; in the existing afterEach (which currently calls vi.unstubAllGlobals()) call cleanup() as well to unmount DOM between tests (import cleanup from the appropriate testing library, e.g., '@testing-library/svelte'). Reference the existing beforeEach/afterEach blocks and the symbols callback, mockSseFn, notificationStream.disconnect, and vi.unstubAllGlobals when making the change.frontend/src/routes/__tests__/Settings.test.ts (1)
7-31:⚠️ Potential issue | 🟡 MinorMock data missing
created_atandupdated_atfields.According to the
UserSettingstype fromtypes.gen.ts, the mock is missing requiredcreated_atandupdated_atstring fields. If the component accesses these fields, tests may behave unexpectedly.Proposed fix
function createMockSettings() { return { user_id: 'user-1', theme: 'dark', timezone: 'UTC', date_format: 'YYYY-MM-DD', time_format: '24h', notifications: { execution_completed: true, execution_failed: false, system_updates: true, security_alerts: false, channels: ['in_app'], }, editor: { font_size: 16, tab_size: 2, use_tabs: true, word_wrap: false, show_line_numbers: true, }, custom_settings: {}, version: 1, + created_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/__tests__/Settings.test.ts` around lines 7 - 31, The mock returned by createMockSettings does not include the required created_at and updated_at string fields from the UserSettings type; update the createMockSettings function to add created_at and updated_at (use valid ISO timestamp strings, e.g. new Date().toISOString() or fixed ISO strings) so the mock matches UserSettings and tests that access those properties won't fail.
🧹 Nitpick comments (5)
frontend/src/stores/__tests__/auth.test.ts (1)
4-26: Path aliases correctly applied.The mock paths now use
$lib/api,$stores/userSettings.svelte, and$lib/user-settings, matching the actual imports inauth.svelte.tsand the Vitest alias configuration.One optional refinement: the coding guidelines recommend using
vi.hoisted()for mock function variables that are referenced insidevi.mock()factories. This ensures deterministic hoisting behavior.,
♻️ Optional: Use vi.hoisted() pattern
+const { mockLoginApi, mockLogoutApi, mockGetProfileApi } = vi.hoisted(() => ({ + mockLoginApi: vi.fn(), + mockLogoutApi: vi.fn(), + mockGetProfileApi: vi.fn(), +})); -const mockLoginApi = vi.fn(); -const mockLogoutApi = vi.fn(); -const mockGetProfileApi = vi.fn(); vi.mock('$lib/api', () => ({ loginApiV1AuthLoginPost: (...args: unknown[]) => mockLoginApi(...args), logoutApiV1AuthLogoutPost: (...args: unknown[]) => mockLogoutApi(...args), getCurrentUserProfileApiV1AuthMeGet: (...args: unknown[]) => mockGetProfileApi(...args), })); +const { mockClearUserSettings } = vi.hoisted(() => ({ + mockClearUserSettings: vi.fn(), +})); -const mockClearUserSettings = vi.fn(); vi.mock('$stores/userSettings.svelte', () => ({ clearUserSettings: () => mockClearUserSettings(), setUserSettings: vi.fn(), userSettingsStore: { settings: null, editorSettings: {} }, })); +const { mockLoadUserSettings } = vi.hoisted(() => ({ + mockLoadUserSettings: vi.fn(), +})); -const mockLoadUserSettings = vi.fn(); vi.mock('$lib/user-settings', () => ({ loadUserSettings: () => mockLoadUserSettings(), }));As per coding guidelines: "Frontend Vitest tests must hoist mocks with
vi.hoisted()beforevi.mock()declarations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/__tests__/auth.test.ts` around lines 4 - 26, Replace the plain vi.fn() mock declarations with hoisted mocks so Vitest deterministically hoists them before the vi.mock factories: change mockLoginApi, mockLogoutApi, mockGetProfileApi, mockClearUserSettings and mockLoadUserSettings to be created via vi.hoisted(() => vi.fn()) and keep using those symbols inside the existing vi.mock(...) factories (loginApiV1AuthLoginPost, logoutApiV1AuthLogoutPost, getCurrentUserProfileApiV1AuthMeGet, clearUserSettings, loadUserSettings) so the mocks are hoisted correctly.frontend/src/lib/editor/__tests__/execution.test.ts (1)
30-34: Consider addingcleanup()inafterEachfor consistency.The coding guidelines specify calling
cleanup()in VitestafterEachfor test files in this path. While this test file doesn't render Svelte components (so cleanup isn't strictly necessary here), adding it maintains consistency across the test suite and future-proofs against component rendering being added later.♻️ Optional: Add afterEach with cleanup
+import { cleanup } from '@testing-library/svelte'; +import { afterEach } from 'vitest'; + // ... existing imports ... describe('createExecutionState', () => { beforeEach(() => { mockCreateExecution.mockReset(); mockSseFn.mockReset(); }); + + afterEach(() => { + cleanup(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/editor/__tests__/execution.test.ts` around lines 30 - 34, Add an afterEach that calls cleanup() alongside the existing mock resets to follow the Vitest guideline; in the describe block for "createExecutionState" add an afterEach that invokes cleanup() (and you may also reset mocks there or keep the existing beforeEach mockReset calls) so the test file calls cleanup() for future Svelte component rendering and consistency with the test suite.frontend/src/__tests__/test-utils.ts (1)
36-37: Remove redundant type assertion.The
as ReturnType<typeof vi.fn>assertion is unnecessary sincevi.mocked(fn)already returns the correct mock type. This was flagged by static analysis.Proposed fix
export function mockApi(fn: (...args: any[]) => any) { - const mock = vi.mocked(fn) as ReturnType<typeof vi.fn>; + const mock = vi.mocked(fn); return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/__tests__/test-utils.ts` around lines 36 - 37, The current mockApi implementation uses a redundant type assertion on the mocked function; update the line in mockApi so that it simply assigns vi.mocked(fn) to mock (i.e., use const mock = vi.mocked(fn);) and remove the trailing "as ReturnType<typeof vi.fn>" cast so the inferred mock type from vi.mocked is used (refer to the mockApi function and vi.mocked usage).frontend/vitest.setup.ts (1)
47-48: Usefor...ofloop instead offorEachwithdelete.The
deleteexpression returns a value, which triggers the Biome lint warning about returning values fromforEachcallbacks. Using afor...ofloop is cleaner.Proposed fix
clear: vi.fn(() => { - Object.keys(localStorageStore).forEach((key) => delete localStorageStore[key]); + for (const key of Object.keys(localStorageStore)) { + delete localStorageStore[key]; + } }),Apply the same pattern to
sessionStorageMock.clear(line 68) and thebeforeEachcleanup (lines 181-182).Also applies to: 67-68, 181-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/vitest.setup.ts` around lines 47 - 48, Replace the forEach + delete pattern with a for...of loop to avoid returning values from the callback: in localStorageMock.clear (the clear: vi.fn(...) that iterates Object.keys(localStorageStore)), change to iterating Object.keys(localStorageStore) with for (const key of ...) and delete localStorageStore[key]; do the same update for sessionStorageMock.clear and the beforeEach cleanup block that currently uses Object.keys(...).forEach(...) so each key is deleted inside a for...of loop.frontend/src/lib/__tests__/user-settings.test.ts (1)
51-53: Addcleanup()call inafterEach.Per coding guidelines,
cleanup()should be called inafterEachto ensure proper DOM cleanup between tests.Proposed fix
+import { cleanup } from '@testing-library/svelte'; + import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';afterEach(() => { vi.restoreAllMocks(); + cleanup(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/__tests__/user-settings.test.ts` around lines 51 - 53, The tests in user-settings.test.ts currently call vi.restoreAllMocks() in afterEach but omit DOM cleanup; update the afterEach block to call cleanup() as well (e.g., afterEach(() => { cleanup(); vi.restoreAllMocks(); })), and ensure the testing library's cleanup function is imported (import { cleanup } from '@testing-library/svelte' or the project's testing-library wrapper) so cleanup() is available; reference the existing afterEach and the cleanup symbol when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/lib/notifications/__tests__/stream.test.ts`:
- Line 8: Add missing test lifecycle cleanup calls: inside the beforeEach block
(where callback = vi.fn() is set) call vi.clearAllMocks() to reset mocks like
mockSseFn/notificationStreamApiV1EventsNotificationsStreamGet, and inside the
afterEach block (after vi.unstubAllGlobals()) call cleanup() to unmount DOM
state; ensure these calls are placed in the existing beforeEach/afterEach
functions so test hygiene is preserved.
In `@frontend/src/routes/admin/__tests__/AdminEvents.test.ts`:
- Around line 88-91: The mock implementations for
browseEventsApiV1AdminEventsBrowsePost (and the other similar mocks) are
directly calling toast.error(...) inside the mock; instead they should only
return an error response so the global API interceptor shows the toast. Replace
the mockImplementation that invokes toast with one that returns an error-shaped
response (use the shared test helper pattern mockApi(...).err() or return {
data: null, error: <ErrorLike> } ) for browseEventsApiV1AdminEventsBrowsePost
and the other affected mocks so component error handling and the global
interceptor drive toast/display behavior rather than the mock itself.
In `@frontend/src/routes/admin/__tests__/AdminExecutions.test.ts`:
- Around line 18-21: Wrap the existing vi.mock(...) call that returns
MockAdminLayout.svelte inside a vi.hoisted() block so the mock is hoisted before
component imports; specifically, replace the standalone vi.mock for
'$routes/admin/AdminLayout.svelte' with a vi.hoisted(() => {
vi.mock('$routes/admin/AdminLayout.svelte', async () => { const { default:
MockLayout } = await
import('$routes/admin/__tests__/mocks/MockAdminLayout.svelte'); return {
default: MockLayout }; }); }); ensuring the vi.hoisted call surrounds the
vi.mock declaration for proper hoisting.
In `@frontend/src/routes/admin/__tests__/AdminSagas.test.ts`:
- Around line 7-10: The vi.mock call registering the AdminLayout mock needs to
be hoisted: wrap the existing vi.mock('$routes/admin/AdminLayout.svelte', ...)
invocation inside vi.hoisted(() => { ... }) so Vitest hoists the mock; keep the
same async import of '$routes/admin/__tests__/mocks/MockAdminLayout.svelte' and
return { default: MockLayout } inside the hoisted callback, preserving the
module string and mock behavior.
In `@frontend/src/routes/admin/__tests__/AdminUsers.test.ts`:
- Around line 16-24: The vi.mock() declarations for the formatter and
AdminLayout are not hoisted; wrap them using vi.hoisted so the mocks evaluate at
module load time: create hoisted mock objects for formatTimestamp and for the
AdminLayout import (referencing formatTimestamp and
AdminLayout/MockAdminLayout.svelte) and then pass those hoisted objects into
vi.mock() calls instead of the current inline factories (i.e., replace the
current vi.mock('$lib/formatters', ...) and
vi.mock('$routes/admin/AdminLayout.svelte', ...) with hoisted versions so mocks
run before imports and component rendering).
- Around line 50-53: The test mocks (e.g.,
vi.mocked(listUsersApiV1AdminUsersGet)) are calling toast.error inside the mock
which short-circuits the component's own error handling; change each mock
implementation to simply return an error-shaped response (e.g., { data: null,
error: ... }) and remove any toast.error calls so the component's unwrap() path
can run and trigger toasts itself; update all similar mocks (the ones around the
other indicated blocks) to follow this pattern and assert the
component-triggered toast instead of the mock doing it.
---
Outside diff comments:
In `@frontend/src/lib/notifications/__tests__/stream.test.ts`:
- Around line 54-62: Add the missing test-hygiene calls: in the existing
beforeEach (where callback, mockSseFn.mockReset and
notificationStream.disconnect are already called) add vi.clearAllMocks() to
reset mocks before each test; in the existing afterEach (which currently calls
vi.unstubAllGlobals()) call cleanup() as well to unmount DOM between tests
(import cleanup from the appropriate testing library, e.g.,
'@testing-library/svelte'). Reference the existing beforeEach/afterEach blocks
and the symbols callback, mockSseFn, notificationStream.disconnect, and
vi.unstubAllGlobals when making the change.
In `@frontend/src/routes/__tests__/Settings.test.ts`:
- Around line 7-31: The mock returned by createMockSettings does not include the
required created_at and updated_at string fields from the UserSettings type;
update the createMockSettings function to add created_at and updated_at (use
valid ISO timestamp strings, e.g. new Date().toISOString() or fixed ISO strings)
so the mock matches UserSettings and tests that access those properties won't
fail.
---
Nitpick comments:
In `@frontend/src/__tests__/test-utils.ts`:
- Around line 36-37: The current mockApi implementation uses a redundant type
assertion on the mocked function; update the line in mockApi so that it simply
assigns vi.mocked(fn) to mock (i.e., use const mock = vi.mocked(fn);) and remove
the trailing "as ReturnType<typeof vi.fn>" cast so the inferred mock type from
vi.mocked is used (refer to the mockApi function and vi.mocked usage).
In `@frontend/src/lib/__tests__/user-settings.test.ts`:
- Around line 51-53: The tests in user-settings.test.ts currently call
vi.restoreAllMocks() in afterEach but omit DOM cleanup; update the afterEach
block to call cleanup() as well (e.g., afterEach(() => { cleanup();
vi.restoreAllMocks(); })), and ensure the testing library's cleanup function is
imported (import { cleanup } from '@testing-library/svelte' or the project's
testing-library wrapper) so cleanup() is available; reference the existing
afterEach and the cleanup symbol when making this change.
In `@frontend/src/lib/editor/__tests__/execution.test.ts`:
- Around line 30-34: Add an afterEach that calls cleanup() alongside the
existing mock resets to follow the Vitest guideline; in the describe block for
"createExecutionState" add an afterEach that invokes cleanup() (and you may also
reset mocks there or keep the existing beforeEach mockReset calls) so the test
file calls cleanup() for future Svelte component rendering and consistency with
the test suite.
In `@frontend/src/stores/__tests__/auth.test.ts`:
- Around line 4-26: Replace the plain vi.fn() mock declarations with hoisted
mocks so Vitest deterministically hoists them before the vi.mock factories:
change mockLoginApi, mockLogoutApi, mockGetProfileApi, mockClearUserSettings and
mockLoadUserSettings to be created via vi.hoisted(() => vi.fn()) and keep using
those symbols inside the existing vi.mock(...) factories
(loginApiV1AuthLoginPost, logoutApiV1AuthLogoutPost,
getCurrentUserProfileApiV1AuthMeGet, clearUserSettings, loadUserSettings) so the
mocks are hoisted correctly.
In `@frontend/vitest.setup.ts`:
- Around line 47-48: Replace the forEach + delete pattern with a for...of loop
to avoid returning values from the callback: in localStorageMock.clear (the
clear: vi.fn(...) that iterates Object.keys(localStorageStore)), change to
iterating Object.keys(localStorageStore) with for (const key of ...) and delete
localStorageStore[key]; do the same update for sessionStorageMock.clear and the
beforeEach cleanup block that currently uses Object.keys(...).forEach(...) so
each key is deleted inside a for...of loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be7917fd-c04b-4e8e-8498-0cdc1dfa2233
📒 Files selected for processing (28)
frontend/src/__tests__/test-utils.tsfrontend/src/components/__tests__/Header.test.tsfrontend/src/components/__tests__/NotificationCenter.test.tsfrontend/src/components/admin/users/__tests__/RateLimitsModal.test.tsfrontend/src/components/editor/__tests__/OutputPanel.test.tsfrontend/src/lib/__tests__/api-interceptors.test.tsfrontend/src/lib/__tests__/user-settings.test.tsfrontend/src/lib/admin/__tests__/pagination.test.tsfrontend/src/lib/admin/stores/__tests__/eventsStore.test.tsfrontend/src/lib/admin/stores/__tests__/executionsStore.test.tsfrontend/src/lib/admin/stores/__tests__/sagasStore.test.tsfrontend/src/lib/editor/__tests__/execution.test.tsfrontend/src/lib/notifications/__tests__/stream.test.tsfrontend/src/routes/__tests__/Editor.test.tsfrontend/src/routes/__tests__/Home.test.tsfrontend/src/routes/__tests__/Login.test.tsfrontend/src/routes/__tests__/Notifications.test.tsfrontend/src/routes/__tests__/Register.test.tsfrontend/src/routes/__tests__/Settings.test.tsfrontend/src/routes/admin/__tests__/AdminEvents.test.tsfrontend/src/routes/admin/__tests__/AdminExecutions.test.tsfrontend/src/routes/admin/__tests__/AdminLayout.test.tsfrontend/src/routes/admin/__tests__/AdminSagas.test.tsfrontend/src/routes/admin/__tests__/AdminSettings.test.tsfrontend/src/routes/admin/__tests__/AdminUsers.test.tsfrontend/src/stores/__tests__/auth.test.tsfrontend/src/stores/__tests__/theme.test.tsfrontend/vitest.setup.ts
💤 Files with no reviewable changes (4)
- frontend/src/components/editor/tests/OutputPanel.test.ts
- frontend/src/routes/tests/Home.test.ts
- frontend/src/routes/admin/tests/AdminLayout.test.ts
- frontend/src/routes/tests/Notifications.test.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/src/lib/admin/tests/pagination.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/stores/__tests__/theme.test.ts (1)
3-11:⚠️ Potential issue | 🟡 MinorHoist mock handles with
vi.hoisted()beforevi.mock().Lines 3-11 declare mocks directly in
vi.mock()factories. Per the required testing patterns, mocks must be created in a hoisted block first, then referenced fromvi.mock().Proposed fix
+const mocks = vi.hoisted(() => ({ + saveUserSettings: vi.fn().mockResolvedValue(true), + authStore: { + isAuthenticated: false, + }, +})); + vi.mock('$lib/user-settings', () => ({ - saveUserSettings: vi.fn().mockResolvedValue(true), + saveUserSettings: mocks.saveUserSettings, })); vi.mock('$stores/auth.svelte', () => ({ - authStore: { - isAuthenticated: false, - }, + authStore: mocks.authStore, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/__tests__/theme.test.ts` around lines 3 - 11, The tests currently define mock implementations inline inside vi.mock factories for saveUserSettings and authStore; instead, create hoisted mock handles using vi.hoisted() (e.g., const saveUserSettingsMock = vi.hoisted(() => vi.fn().mockResolvedValue(true)) and const authStoreMock = vi.hoisted(() => ({ authStore: { isAuthenticated: false } }))) and then reference those handles inside vi.mock factories (vi.mock('$lib/user-settings', () => ({ saveUserSettings: saveUserSettingsMock })) and vi.mock('$stores/auth.svelte', () => authStoreMock)); update the references in theme.test.ts to use these hoisted handles so mocks are created before imports.
♻️ Duplicate comments (1)
.github/actions/e2e-ready/action.yml (1)
43-46:⚠️ Potential issue | 🟠 MajorTimeout outcomes are still masked by
|| true, making hang failures opaque.On Line 43, Line 84, Line 88, and Line 98,
timeout ... tail ... || truehides whether the wait timed out (exit 124). The step then fails later via exit-file checks, but without a clear timeout error source.🔧 Suggested pattern (apply to each wait block)
- timeout 120 tail --pid="$(cat /tmp/kueue-download.pid)" -f /dev/null 2>/dev/null || true + if ! timeout 120 tail --pid="$(cat /tmp/kueue-download.pid)" -f /dev/null 2>/dev/null; then + echo "::error::Timed out waiting for kueue download background task" + exit 1 + fi cat /tmp/kueue-download.log 2>/dev/null || true [ "$(cat /tmp/kueue-download.exit)" = "0" ]Use this script to confirm all masked timeout sites before patching:
#!/bin/bash set -euo pipefail rg -n -C1 'timeout\s+120\s+tail\s+--pid=.*\|\|\s*true' .github/actions/e2e-ready/action.ymlExpected result: matches on the current wait blocks (kueue/crictl/infra), which should be replaced with explicit timeout handling.
Also applies to: 84-90, 98-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/e2e-ready/action.yml around lines 43 - 46, The wait blocks use the pattern "timeout 120 tail --pid=... -f /dev/null || true" which masks timeout exit code 124; update each such block (the kueue/crictl/infra wait invocations and the ones that touch "/tmp/kueue-download.exit" etc.) to remove the unconditional "|| true", capture the timeout command's exit status, and explicitly handle a 124 timeout by logging a clear timeout error and propagating a non-zero exit (and/or writing the appropriate exit file) so the pipeline fails with an explicit timeout instead of being masked; ensure the new logic still preserves existing successful-wait behavior.
🧹 Nitpick comments (5)
frontend/src/stores/__tests__/auth.test.ts (2)
42-63: Movevi.clearAllMocks()fromafterEachtobeforeEach.The coding guidelines specify calling
vi.clearAllMocks()inbeforeEach, notafterEach. This ensures a clean state before each test runs.♻️ Proposed refactor
beforeEach(async () => { + vi.clearAllMocks(); sessionStorageData = {}; vi.mocked(sessionStorage.getItem).mockImplementation((key: string) => sessionStorageData[key] ?? null); // ... rest of beforeEach }); afterEach(() => { - vi.clearAllMocks(); });As per coding guidelines: "Call
cleanup()in VitestafterEach,vi.clearAllMocks()inbeforeEach".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/__tests__/auth.test.ts` around lines 42 - 63, Move the vi.clearAllMocks() call out of the afterEach block and into the beforeEach setup so mocks are cleared before each test; update the test file's lifecycle hooks by removing vi.clearAllMocks() from the afterEach block and adding a vi.clearAllMocks() invocation at the start of the existing beforeEach (where sessionStorage mocks and mock* resets are performed) to conform to the guideline that vi.clearAllMocks() runs in beforeEach rather than afterEach.
4-26: Mock functions should be hoisted withvi.hoisted().The mock functions (
mockLoginApi,mockLogoutApi,mockGetProfileApi,mockClearUserSettings,mockLoadUserSettings) are declared at module scope withoutvi.hoisted(). Per coding guidelines, Vitest tests must hoist mocks beforevi.mock()declarations to ensure proper execution order.♻️ Proposed refactor using vi.hoisted()
-const mockLoginApi = vi.fn(); -const mockLogoutApi = vi.fn(); -const mockGetProfileApi = vi.fn(); +const { mockLoginApi, mockLogoutApi, mockGetProfileApi } = vi.hoisted(() => ({ + mockLoginApi: vi.fn(), + mockLogoutApi: vi.fn(), + mockGetProfileApi: vi.fn(), +})); vi.mock('$lib/api', () => ({ loginApiV1AuthLoginPost: (...args: unknown[]) => mockLoginApi(...args), logoutApiV1AuthLogoutPost: (...args: unknown[]) => mockLogoutApi(...args), getCurrentUserProfileApiV1AuthMeGet: (...args: unknown[]) => mockGetProfileApi(...args), })); -// Mock clearUserSettings (static import in auth.svelte.ts) -const mockClearUserSettings = vi.fn(); +const { mockClearUserSettings } = vi.hoisted(() => ({ + mockClearUserSettings: vi.fn(), +})); + vi.mock('$stores/userSettings.svelte', () => ({ clearUserSettings: () => mockClearUserSettings(), setUserSettings: vi.fn(), userSettingsStore: { settings: null, editorSettings: {} }, })); -// Mock loadUserSettings (dynamic import in auth.svelte.ts) -const mockLoadUserSettings = vi.fn(); +const { mockLoadUserSettings } = vi.hoisted(() => ({ + mockLoadUserSettings: vi.fn(), +})); + vi.mock('$lib/user-settings', () => ({ loadUserSettings: () => mockLoadUserSettings(), }));As per coding guidelines: "Frontend Vitest tests must hoist mocks with
vi.hoisted()beforevi.mock()declarations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/__tests__/auth.test.ts` around lines 4 - 26, The test-level mock functions (mockLoginApi, mockLogoutApi, mockGetProfileApi, mockClearUserSettings, mockLoadUserSettings) must be created with vi.hoisted() so they are defined before vi.mock() calls; replace the current top-level const declarations with hoisted mocks (e.g., const mockLoginApi = vi.hoisted(() => vi.fn()) etc.), keep referencing the same identifiers used by the vi.mock() callbacks (loginApiV1AuthLoginPost, logoutApiV1AuthLogoutPost, getCurrentUserProfileApiV1AuthMeGet, clearUserSettings, loadUserSettings) so the existing mock implementations continue to call those hoisted functions.frontend/src/routes/__tests__/Editor.test.ts (1)
21-28: Consider moving imports to the top of the file.The API function imports are placed after the
createMockLimitsfunction definition, which is unconventional. While JavaScript hoisting makes this work, grouping all imports at the top improves readability and follows standard conventions.Suggested reordering
Move these imports to appear right after line 6 (after other imports, before any function definitions):
import Editor from '$routes/Editor.svelte'; + +import { + getK8sResourceLimitsApiV1K8sLimitsGet, + getExampleScriptsApiV1ExampleScriptsGet, + listSavedScriptsApiV1ScriptsGet, + createSavedScriptApiV1ScriptsPost, + updateSavedScriptApiV1ScriptsScriptIdPut, + deleteSavedScriptApiV1ScriptsScriptIdDelete, +} from '$lib/api'; function createMockLimits() {Then remove lines 21-29.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/__tests__/Editor.test.ts` around lines 21 - 28, Move the API function imports (getK8sResourceLimitsApiV1K8sLimitsGet, getExampleScriptsApiV1ExampleScriptsGet, listSavedScriptsApiV1ScriptsGet, createSavedScriptApiV1ScriptsPost, updateSavedScriptApiV1ScriptsScriptIdPut, deleteSavedScriptApiV1ScriptsScriptIdDelete) so they appear with the other imports at the top of the file (i.e., immediately after the existing import block and before the createMockLimits function definition), and remove the duplicate import block currently placed after createMockLimits to keep imports grouped and consistent.frontend/vitest.setup.ts (2)
171-175: Addvi.clearAllMocks()to reset mock call state between tests.The global mocks (
$lib/api, router, toast, interceptors) defined at module level will accumulate call counts across tests without this. Add it to ensure each test starts with fresh mock state:Suggested fix
beforeEach(() => { + vi.clearAllMocks(); Object.keys(localStorageStore).forEach((key) => delete localStorageStore[key]); Object.keys(sessionStorageStore).forEach((key) => delete sessionStorageStore[key]); cleanup(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/vitest.setup.ts` around lines 171 - 175, Add a call to vi.clearAllMocks() inside the existing beforeEach so global mocks are reset between tests; update the beforeEach block that currently clears localStorageStore/sessionStorageStore and calls cleanup() (the function named beforeEach, and the variables localStorageStore, sessionStorageStore, and cleanup) to invoke vi.clearAllMocks() (preferably at the start) to reset mock call state for module-level mocks like $lib/api, router, toast, and interceptors.
44-44: RefactorforEachcallbacks to usefor...ofloops for better code clarity.Lines 44, 63, 172, and 173 use
Object.keys(...).forEach((key) => delete store[key]). While this works, using afor...ofloop is more idiomatic and explicit:Suggested refactor
- Object.keys(localStorageStore).forEach((key) => delete localStorageStore[key]); + for (const key of Object.keys(localStorageStore)) { + delete localStorageStore[key]; + } - Object.keys(sessionStorageStore).forEach((key) => delete sessionStorageStore[key]); + for (const key of Object.keys(sessionStorageStore)) { + delete sessionStorageStore[key]; + } - Object.keys(localStorageStore).forEach((key) => delete localStorageStore[key]); - Object.keys(sessionStorageStore).forEach((key) => delete sessionStorageStore[key]); + for (const key of Object.keys(localStorageStore)) { + delete localStorageStore[key]; + } + for (const key of Object.keys(sessionStorageStore)) { + delete sessionStorageStore[key]; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/vitest.setup.ts` at line 44, Replace the use of Object.keys(...).forEach((key) => delete store[key]) with an explicit for...of loop for clarity: locate the occurrences (e.g., the localStorageStore cleanup in vitest.setup.ts and the other similar cleanups referenced at lines 63, 172, 173) and change the callback style to iterate keys via for (const key of Object.keys(store)) { delete store[key]; } so each store cleanup (localStorageStore and the other store variables) uses a for...of loop instead of forEach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/routes/__tests__/Editor.test.ts`:
- Line 2: The tests import from '@testing-library/svelte' but do not import or
call cleanup; update the import to include cleanup and modify the test lifecycle
to call cleanup() in afterEach and vi.clearAllMocks() in beforeEach so
components are unmounted and mocks reset; locate the import line bringing in
render/screen/waitFor and add cleanup, then add/adjust the beforeEach to call
vi.clearAllMocks() and the afterEach to call cleanup() (referencing the symbols
render, cleanup, beforeEach, afterEach, vi.clearAllMocks).
---
Outside diff comments:
In `@frontend/src/stores/__tests__/theme.test.ts`:
- Around line 3-11: The tests currently define mock implementations inline
inside vi.mock factories for saveUserSettings and authStore; instead, create
hoisted mock handles using vi.hoisted() (e.g., const saveUserSettingsMock =
vi.hoisted(() => vi.fn().mockResolvedValue(true)) and const authStoreMock =
vi.hoisted(() => ({ authStore: { isAuthenticated: false } }))) and then
reference those handles inside vi.mock factories (vi.mock('$lib/user-settings',
() => ({ saveUserSettings: saveUserSettingsMock })) and
vi.mock('$stores/auth.svelte', () => authStoreMock)); update the references in
theme.test.ts to use these hoisted handles so mocks are created before imports.
---
Duplicate comments:
In @.github/actions/e2e-ready/action.yml:
- Around line 43-46: The wait blocks use the pattern "timeout 120 tail --pid=...
-f /dev/null || true" which masks timeout exit code 124; update each such block
(the kueue/crictl/infra wait invocations and the ones that touch
"/tmp/kueue-download.exit" etc.) to remove the unconditional "|| true", capture
the timeout command's exit status, and explicitly handle a 124 timeout by
logging a clear timeout error and propagating a non-zero exit (and/or writing
the appropriate exit file) so the pipeline fails with an explicit timeout
instead of being masked; ensure the new logic still preserves existing
successful-wait behavior.
---
Nitpick comments:
In `@frontend/src/routes/__tests__/Editor.test.ts`:
- Around line 21-28: Move the API function imports
(getK8sResourceLimitsApiV1K8sLimitsGet, getExampleScriptsApiV1ExampleScriptsGet,
listSavedScriptsApiV1ScriptsGet, createSavedScriptApiV1ScriptsPost,
updateSavedScriptApiV1ScriptsScriptIdPut,
deleteSavedScriptApiV1ScriptsScriptIdDelete) so they appear with the other
imports at the top of the file (i.e., immediately after the existing import
block and before the createMockLimits function definition), and remove the
duplicate import block currently placed after createMockLimits to keep imports
grouped and consistent.
In `@frontend/src/stores/__tests__/auth.test.ts`:
- Around line 42-63: Move the vi.clearAllMocks() call out of the afterEach block
and into the beforeEach setup so mocks are cleared before each test; update the
test file's lifecycle hooks by removing vi.clearAllMocks() from the afterEach
block and adding a vi.clearAllMocks() invocation at the start of the existing
beforeEach (where sessionStorage mocks and mock* resets are performed) to
conform to the guideline that vi.clearAllMocks() runs in beforeEach rather than
afterEach.
- Around line 4-26: The test-level mock functions (mockLoginApi, mockLogoutApi,
mockGetProfileApi, mockClearUserSettings, mockLoadUserSettings) must be created
with vi.hoisted() so they are defined before vi.mock() calls; replace the
current top-level const declarations with hoisted mocks (e.g., const
mockLoginApi = vi.hoisted(() => vi.fn()) etc.), keep referencing the same
identifiers used by the vi.mock() callbacks (loginApiV1AuthLoginPost,
logoutApiV1AuthLogoutPost, getCurrentUserProfileApiV1AuthMeGet,
clearUserSettings, loadUserSettings) so the existing mock implementations
continue to call those hoisted functions.
In `@frontend/vitest.setup.ts`:
- Around line 171-175: Add a call to vi.clearAllMocks() inside the existing
beforeEach so global mocks are reset between tests; update the beforeEach block
that currently clears localStorageStore/sessionStorageStore and calls cleanup()
(the function named beforeEach, and the variables localStorageStore,
sessionStorageStore, and cleanup) to invoke vi.clearAllMocks() (preferably at
the start) to reset mock call state for module-level mocks like $lib/api,
router, toast, and interceptors.
- Line 44: Replace the use of Object.keys(...).forEach((key) => delete
store[key]) with an explicit for...of loop for clarity: locate the occurrences
(e.g., the localStorageStore cleanup in vitest.setup.ts and the other similar
cleanups referenced at lines 63, 172, 173) and change the callback style to
iterate keys via for (const key of Object.keys(store)) { delete store[key]; } so
each store cleanup (localStorageStore and the other store variables) uses a
for...of loop instead of forEach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37864b5e-d847-48c3-ba3a-e48517845d4a
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/actions/e2e-ready/action.ymlfrontend/package.jsonfrontend/src/components/__tests__/ErrorDisplay.test.tsfrontend/src/components/__tests__/Footer.test.tsfrontend/src/components/__tests__/Header.test.tsfrontend/src/components/__tests__/NotificationCenter.test.tsfrontend/src/components/__tests__/Spinner.test.tsfrontend/src/components/admin/users/__tests__/RateLimitsModal.test.tsfrontend/src/lib/editor/__tests__/languages.test.tsfrontend/src/routes/__tests__/Editor.test.tsfrontend/src/stores/__tests__/auth.test.tsfrontend/src/stores/__tests__/theme.test.tsfrontend/src/utils/__tests__/meta.test.tsfrontend/vitest.setup.ts
💤 Files with no reviewable changes (5)
- frontend/src/lib/editor/tests/languages.test.ts
- frontend/src/components/tests/Spinner.test.ts
- frontend/src/components/tests/ErrorDisplay.test.ts
- frontend/src/components/tests/Footer.test.ts
- frontend/src/utils/tests/meta.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/admin/users/tests/RateLimitsModal.test.ts
| @@ -1,28 +1,33 @@ | |||
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | |||
| import { render, screen, waitFor } from '@testing-library/svelte'; | |||
There was a problem hiding this comment.
Missing cleanup() call in afterEach.
Per coding guidelines, Vitest tests should call cleanup() in afterEach to properly unmount components and prevent test pollution between tests. The cleanup function also needs to be imported from @testing-library/svelte.
Proposed fix
Update the import on line 2:
-import { render, screen, waitFor } from '@testing-library/svelte';
+import { cleanup, render, screen, waitFor } from '@testing-library/svelte';Update afterEach on line 107:
- afterEach(() => vi.unstubAllGlobals());
+ afterEach(() => {
+ cleanup();
+ vi.unstubAllGlobals();
+ });Based on learnings: "Call cleanup() in Vitest afterEach, vi.clearAllMocks() in beforeEach"
Also applies to: 107-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/routes/__tests__/Editor.test.ts` at line 2, The tests import
from '@testing-library/svelte' but do not import or call cleanup; update the
import to include cleanup and modify the test lifecycle to call cleanup() in
afterEach and vi.clearAllMocks() in beforeEach so components are unmounted and
mocks reset; locate the import line bringing in render/screen/waitFor and add
cleanup, then add/adjust the beforeEach to call vi.clearAllMocks() and the
afterEach to call cleanup() (referencing the symbols render, cleanup,
beforeEach, afterEach, vi.clearAllMocks).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Speed up CI test runs by parallelizing E2E setup and cutting Python env syncs. Frontend tests now run on happy-dom with isolation, centralized mocks, and cleaner select/checkbox binding.
Written for commit 6775931. Summary will update on new commits.
Summary by CodeRabbit
Chores
New Features
Tests