mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): wire error-banner guard into e2e fixtures
The guard wasn't firing because Electron tests create their own page
via app.firstWindow(), not via Playwright's default page fixture. The
base.afterEach's page fixture was undefined for Electron tests.
Fix: export installErrorBannerGuard + collectErrorBanners from
e2e/test.ts, call installErrorBannerGuard from launchDesktop() and
setupPackagedApp() in fixtures.ts (both firstWindow() call sites),
and use activePage (set by the guard) in afterEach instead of the
default page fixture.
Verified: the guard now catches the 'Resume failed' error banner
('handler error: str object has no attribute get') that appears
during the large-session-reload e2e test.
This commit is contained in:
parent
5604e1256f
commit
464a0645e7
3 changed files with 37 additions and 9 deletions
|
|
@ -28,6 +28,7 @@ import * as path from 'node:path'
|
|||
import { _electron, type ElectronApplication, type Page } from '@playwright/test'
|
||||
|
||||
import { startMockServer } from './mock-server'
|
||||
import { installErrorBannerGuard } from './test'
|
||||
|
||||
const DESKTOP_ROOT = path.resolve(import.meta.dirname, '..')
|
||||
const REPO_ROOT = path.resolve(DESKTOP_ROOT, '..', '..')
|
||||
|
|
@ -305,6 +306,10 @@ export async function launchDesktop(
|
|||
|
||||
const page = await app.firstWindow()
|
||||
|
||||
// Install the error-banner guard so any [role="alert"] that appears
|
||||
// during a test is collected and surfaced in afterEach.
|
||||
installErrorBannerGuard(page)
|
||||
|
||||
return { app, page }
|
||||
}
|
||||
|
||||
|
|
@ -514,6 +519,7 @@ export async function setupPackagedApp(): Promise<PackagedAppFixture> {
|
|||
})
|
||||
|
||||
const page = await app.firstWindow()
|
||||
installErrorBannerGuard(page)
|
||||
|
||||
return {
|
||||
app,
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ import { spawnSync } from 'node:child_process'
|
|||
import { readFileSync } from 'node:fs'
|
||||
import * as path from 'node:path'
|
||||
|
||||
import { _electron, expect, type ElectronApplication, type Page, test } from './test'
|
||||
import { _electron, expect, installErrorBannerGuard, type ElectronApplication, type Page, test } from './test'
|
||||
|
||||
import {
|
||||
buildAppEnv,
|
||||
|
|
@ -111,6 +111,10 @@ async function setupSeededMockBackend(): Promise<SeededMockBackendFixture> {
|
|||
const env = buildAppEnv(sandbox)
|
||||
const { app, page } = await launchDesktop(env)
|
||||
|
||||
// Install the error-banner guard on the custom page (launchDesktop
|
||||
// already calls this, but call again in case the page was replaced).
|
||||
installErrorBannerGuard(page)
|
||||
|
||||
return {
|
||||
app,
|
||||
page,
|
||||
|
|
|
|||
|
|
@ -23,8 +23,13 @@ let activePage: Page | null = null
|
|||
* Install the error-banner guard on a page. Watches for `[role="alert"]`
|
||||
* elements appearing in the DOM. When one is found, records its text
|
||||
* content for the afterEach assertion.
|
||||
*
|
||||
* Exported so e2e fixture functions (which create pages via _electron.launch)
|
||||
* can install the guard on their custom pages — the default Playwright `page`
|
||||
* fixture override only catches pages created by Playwright itself, not
|
||||
* pages created by the test's own Electron launch.
|
||||
*/
|
||||
function installErrorBannerGuard(page: Page): void {
|
||||
export function installErrorBannerGuard(page: Page): void {
|
||||
activePage = page
|
||||
|
||||
// Clear any errors from a previous test when a new page is created.
|
||||
|
|
@ -69,9 +74,10 @@ function installErrorBannerGuard(page: Page): void {
|
|||
|
||||
/**
|
||||
* Check for error banners that appeared during the test. Called in
|
||||
* afterEach via the custom fixture below.
|
||||
* afterEach via the custom fixture below. Also exported so specs that
|
||||
* manage their own page lifecycle can call it directly.
|
||||
*/
|
||||
async function collectErrorBanners(page: Page | null): Promise<string[]> {
|
||||
export async function collectErrorBanners(page: Page | null): Promise<string[]> {
|
||||
if (!page) {
|
||||
return []
|
||||
}
|
||||
|
|
@ -110,12 +116,18 @@ export const test = base.extend({
|
|||
})
|
||||
|
||||
// afterEach: fail the test if any error banners appeared.
|
||||
base.afterEach(async ({ page }, testInfo) => {
|
||||
const errors = await collectErrorBanners(page ?? activePage)
|
||||
// Always fires — even if the test already failed for another reason.
|
||||
// An error banner often IS the root cause (e.g. "resume failed" from a
|
||||
// backend bug), and suppressing it when the test also fails on an
|
||||
// assertion hides the real problem.
|
||||
//
|
||||
// Uses `activePage` (set by installErrorBannerGuard) instead of the
|
||||
// default `page` fixture — Electron tests create their own page via
|
||||
// app.firstWindow(), so the default `page` fixture is undefined.
|
||||
base.afterEach(async ({}, testInfo) => {
|
||||
const errors = await collectErrorBanners(activePage)
|
||||
|
||||
if (errors.length > 0 && testInfo.status !== 'failed') {
|
||||
// Only fail if the test didn't already fail on its own — we don't
|
||||
// want to mask the original assertion error with our banner check.
|
||||
if (errors.length > 0) {
|
||||
throw new Error(
|
||||
`Error banner(s) appeared during test "${testInfo.title}":\n` +
|
||||
errors.map(e => ` • ${e}`).join('\n'),
|
||||
|
|
@ -123,4 +135,10 @@ base.afterEach(async ({ page }, testInfo) => {
|
|||
}
|
||||
})
|
||||
|
||||
// Reset for the next test file.
|
||||
base.afterAll(async () => {
|
||||
seenErrors.length = 0
|
||||
activePage = null
|
||||
})
|
||||
|
||||
export { expect, type Page, type ElectronApplication, _electron }
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue