This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6600-14c265cb912b75f69c461fe3c95744f0b3196e42 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 2a152549ed68c726ad1ab44925b9bc04e7f38c04 Author: Meng Wang <[email protected]> AuthorDate: Mon Jul 20 12:10:50 2026 -0700 fix(frontend): fork test ProxyZone from the root zone to bound nesting (#6600) ### What changes were proposed in this PR? Frontend unit tests intermittently failed on CI with `RangeError: Maximum call stack size exceeded` in unrelated specs (e.g. `notification.service.spec.ts`, `filters-instructions.component.spec.ts`). The cause is in `frontend/src/test-zone-setup.ts`, which wraps Vitest's `it`/`test` so each spec body runs inside an Angular ProxyZone (needed for `fakeAsync`/`waitForAsync`). It forked a new ProxyZone from `Zone.current` for every test. When an async spec resolves, its continuation can leave the forked proxy as the current zone, so the next fork nests inside it; across the many spec files a Vitest worker runs, the proxy chain grows without bound and eventually overflows the stack inside the `ProxyZoneSpec.onInvoke -> _ZoneDelegate.invoke` delegate chain. Because the depth reached depends on how Vitest packs test files onto workers, the failure is flaky and OS-dependent (observed on `macos-latest` while `ubuntu-latest`/`windows-latest` pass on the same commit), and it can evict an otherwise-green PR from the merge queue. The fix aligns `test-zone-setup.ts` with zone.js's own framework integrations: fork a single ProxyZone from `Zone.root` once, reuse it for every test, and reset its delegate between tests. Forking from the root zone keeps the proxy exactly one level deep regardless of prior test state, so the chain can no longer grow. ### Any related issues, documentation, discussions? Closes #6593. ### How was this PR tested? Added `frontend/src/test-zone-setup.spec.ts`, which asserts the proxy is present and forked directly under the root zone (bounded depth) and that `fakeAsync`/`waitForAsync` still work through the wrapper. Ran all ProxyZone-sensitive specs (every `fakeAsync`/`waitForAsync` spec) locally with and without the change: the new tests pass and no existing test regresses. The flake is not deterministically reproducible from a single spec, so it is not reproduced as a unit test. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8) --- frontend/src/test-zone-setup.spec.ts | 63 ++++++++++++++++++++++++++++++++++++ frontend/src/test-zone-setup.ts | 51 +++++++++++++++++++++++++---- 2 files changed, 107 insertions(+), 7 deletions(-) diff --git a/frontend/src/test-zone-setup.spec.ts b/frontend/src/test-zone-setup.spec.ts new file mode 100644 index 0000000000..71e944cb68 --- /dev/null +++ b/frontend/src/test-zone-setup.spec.ts @@ -0,0 +1,63 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { fakeAsync, tick, waitForAsync } from "@angular/core/testing"; + +/** + * Guards the ProxyZone that `test-zone-setup.ts` installs around every spec. + * + * The flake it fixes (RangeError from unbounded proxy nesting, see + * apache/texera#6593) only surfaces once enough spec files have run on one + * Vitest worker, so it is not reproducible from a single spec. These tests + * instead pin the invariant that prevents it: the proxy is forked directly + * from the root zone (exactly one level deep) and stays functional for + * Angular's fakeAsync/waitForAsync, both of which require an active ProxyZone. + */ + +// `Zone` is a global installed by `zone.js/testing`. Declare the slice used here. +declare const Zone: { + root: unknown; + current: { parent: unknown; get: (key: string) => unknown }; +}; + +describe("test-zone-setup", () => { + it("runs each spec body inside a ProxyZone forked directly from the root zone", () => { + // A ProxyZoneSpec must be in scope, or fakeAsync/waitForAsync would throw + // "Expected to be running in 'ProxyZone'". + expect(Zone.current.get("ProxyZoneSpec")).toBeTruthy(); + // Forked from Zone.root => parent is the root zone => depth is bounded to + // one regardless of how many spec files ran before this one. + expect(Zone.current.parent).toBe(Zone.root); + }); + + it("supports fakeAsync, which requires an active ProxyZone", fakeAsync(() => { + let fired = false; + setTimeout(() => (fired = true), 100); + expect(fired).toBe(false); + tick(100); + expect(fired).toBe(true); + })); + + it("supports waitForAsync, which requires an active ProxyZone", waitForAsync(() => { + // waitForAsync calls ProxyZoneSpec.assertPresent() and swaps the proxy's + // delegate; reaching the assertion at all proves the proxy is present and + // its delegate is restored cleanly afterwards. + Promise.resolve().then(() => expect(true).toBe(true)); + })); +}); diff --git a/frontend/src/test-zone-setup.ts b/frontend/src/test-zone-setup.ts index c6670cda56..8f047f433e 100644 --- a/frontend/src/test-zone-setup.ts +++ b/frontend/src/test-zone-setup.ts @@ -23,20 +23,57 @@ * call chain, Angular's `fakeAsync` throws * `Expected to be running in 'ProxyZone'`. * - * Wrap Vitest's `it` so each test body runs inside a freshly-forked - * ProxyZone. This is a setupFile (referenced from `vitest.config.ts`), - * so it executes once per test file before any spec body runs. + * Wrap Vitest's `it` so each test body runs inside a ProxyZone. The proxy + * is forked ONCE from the ROOT zone and reused for every test, with its + * delegate reset between tests — the same shape zone.js uses for its own + * jasmine/jest integrations and its shared-proxy helper. + * + * Forking from `Zone.root` (rather than `Zone.current`, as before) is what + * keeps the proxy exactly one level deep. When an async spec resolves, its + * continuation can leave the forked ProxyZone as `Zone.current`; the next + * `Zone.current.fork(...)` then nested a proxy inside that one, and across + * the many spec files a Vitest worker runs the chain grew without bound. + * Every `zone.run()` recurses through the whole + * `ProxyZoneSpec.onInvoke -> _ZoneDelegate.invoke` delegate chain, so once + * it is deep enough the stack overflows with `RangeError: Maximum call + * stack size exceeded` in whichever unrelated spec happens to be running. + * See apache/texera#6593. + * + * This is a setupFile (referenced from `vitest.config.ts`), so it executes + * once per test file before any spec body runs. */ import "zone.js/testing"; +type ProxyZone = { run: <T>(fn: () => T) => T }; +type ProxyZoneSpecInstance = { resetDelegate: () => void }; + type ZoneType = { - current: { fork: (spec: object) => { run: <T>(fn: () => T) => T } }; - ProxyZoneSpec: new () => object; + root: { fork: (spec: object) => ProxyZone }; + ProxyZoneSpec: new () => ProxyZoneSpecInstance; }; declare const Zone: ZoneType; -const ProxyZoneSpec = (Zone as unknown as { ProxyZoneSpec: new () => object }).ProxyZoneSpec; +const ProxyZoneSpec = Zone.ProxyZoneSpec; + +// Fork a single ProxyZone from the root zone and reuse it for every test. +let sharedProxyZoneSpec: ProxyZoneSpecInstance | null = null; +let sharedProxyZone: ProxyZone | null = null; + +function getProxyZone(): ProxyZone { + let spec = sharedProxyZoneSpec; + let zone = sharedProxyZone; + if (!spec || !zone) { + spec = new ProxyZoneSpec(); + zone = Zone.root.fork(spec); + sharedProxyZoneSpec = spec; + sharedProxyZone = zone; + } + // Clear any delegate a prior test (e.g. one that threw inside fakeAsync) + // may have left set, so each test starts from a clean proxy state. + spec.resetDelegate(); + return zone; +} type ItFn = (name: string, fn?: (...args: unknown[]) => unknown, timeout?: number) => unknown; @@ -47,7 +84,7 @@ function wrapInProxyZone<T extends ItFn>(target: T): T { name, function wrapper(this: unknown, ...args: unknown[]) { return new Promise<void>((resolve, reject) => { - const zone = Zone.current.fork(new ProxyZoneSpec()); + const zone = getProxyZone(); zone.run(() => { try { const result = fn.apply(this, args);
