mengw15 commented on code in PR #6600:
URL: https://github.com/apache/texera/pull/6600#discussion_r3613819413
##########
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;
+}
Review Comment:
The repo has no `it.concurrent`/`test.concurrent`, and Vitest runs tests
sequentially within a worker (files parallelize across workers, each with its
own `Zone` global), so this proxy is never shared by two tests at once. It's
the same shared-ProxyZone + `resetDelegate` shape zone.js uses for its own
jasmine/jest integrations; forking per-test wouldn't add real safety since
Angular's `TestBed`/`fakeAsync` aren't concurrency-safe either. Happy to
revisit if we ever adopt concurrent specs.
##########
frontend/src/test-zone-setup.ts:
##########
@@ -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();
Review Comment:
Same as the line-76 thread — no concurrent specs in the repo and Vitest is
sequential within a worker, so the shared proxy is never contended.
##########
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", () => {
Review Comment:
`setupFiles` loads this globally and every spec relies on it — that's the
standard Vitest pattern, and validating the setup as loaded by the real config
is exactly what this spec is for. Explicitly importing the module would re-run
it and double-wrap `it` unless guarded, adding a failure mode for a runner we
don't use. The `expect(Zone.current.get("ProxyZoneSpec")).toBeTruthy()`
assertion already fails loudly if the setup didn't load.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
