Merge pull request #5097 from cardstack/worktree-flaky-lint-memory-test

Measure retained heap in lint memory test to remove flakiness

Author: habdelra

packages/realm-server/tests/helpers/prettier-test-utils.ts

@@ -1,5 +1,57 @@
 // Test utilities for prettier formatting tests
 import { performance } from 'perf_hooks';
+import * as v8 from 'v8';
+import * as vm from 'vm';
+
+// Resolved once and cached: the test runner does not start Node with
+// `--expose-gc`, so `globalThis.gc` is normally absent and we synthesize the
+// function through the V8 flag API. Resolving lazily and caching means we
+// create at most one throwaway VM context for the whole suite rather than one
+// per `forceGc` call (a per-call context would itself allocate and perturb the
+// very heap reading the helper exists to make trustworthy). `null` records a
+// completed resolution that produced no usable function.
+let resolvedGc: (() => void) | null | undefined;
+
+function resolveGc(): (() => void) | null {
+  if (resolvedGc !== undefined) {
+    return resolvedGc;
+  }
+  let gc = (globalThis as { gc?: () => void }).gc;
+  if (typeof gc !== 'function') {
+    try {
+      v8.setFlagsFromString('--expose-gc');
+      // Evaluating `gc` throws ReferenceError if the flag didn't take effect
+      // (e.g. an unsupported V8 build); treat that as "no GC available".
+      gc = vm.runInNewContext('gc') as () => void;
+    } catch {
+      gc = undefined;
+    } finally {
+      v8.setFlagsFromString('--no-expose-gc');
+    }
+  }
+  resolvedGc = typeof gc === 'function' ? gc : null;
+  return resolvedGc;
+}
+
+/**
+ * Forces a full garbage collection so that a subsequent
+ * `process.memoryUsage().heapUsed` reading reflects retained memory rather
+ * than uncollected transient garbage. Returns true if a collection was
+ * actually performed, false if no GC function could be resolved — callers can
+ * branch on the result to decide how much to trust the measurement.
+ *
+ * Two passes: the first promotes/collects the young generation, the second
+ * collects what the first pass made unreachable, so the reading settles.
+ */
+export function forceGc(): boolean {
+  const gc = resolveGc();
+  if (!gc) {
+    return false;
+  }
+  gc();
+  gc();
+  return true;
+}
 
 interface FormattingTestCase {
   name: string;

packages/realm-server/tests/realm-endpoints/lint-test.ts

@@ -8,6 +8,7 @@ import {
   createConcurrentTestData,
   createErrorTestCases,
   createPerformanceAssertion,
+  forceGc,
 } from '../helpers/prettier-test-utils';
 import '@cardstack/runtime-common/helpers/code-equality-assertion';
 
@@ -341,27 +342,43 @@ export class MyCard extends CardDef {
     });
 
     test('memory usage during lint operations', async function (assert) {
-      const initialMemory = process.memoryUsage().heapUsed;
-
       const testSource = `import { CardDef } from 'https://cardstack.com/base/card-api';
 export class MyCard extends CardDef {
   @field name = contains(StringField);
 }`;
 
+      const lintOnce = () =>
+        request
+          .post('/_lint')
+          .set(
+            'Authorization',
+            `Bearer ${createJWT(testRealm, 'john', ['read', 'write'])}`,
+          )
+          .set('X-HTTP-Method-Override', 'QUERY')
+          .set('Accept', 'application/json')
+          .send(testSource);
+
+      // Warm up first so one-time, legitimately-retained initialization (eslint
+      // rule definitions, prettier plugins, module caches) is established before
+      // the baseline reading and isn't misattributed to the measured loop.
+      const warmup = await lintOnce();
+      assert.strictEqual(
+        warmup.status,
+        200,
+        'warm-up lint request should succeed so the baseline is a steady-state lint path',
+      );
+
+      // Force a collection before each reading so the delta reflects retained
+      // memory, not transient garbage that GC simply hasn't reclaimed yet.
+      // Without this the reading is dominated by collectible allocations from
+      // ten concurrent lint requests, which is noise, not a leak signal.
+      const gcForced = forceGc();
+      const initialMemory = process.memoryUsage().heapUsed;
+
       // Run multiple lint operations to test memory usage
       const operations = [];
       for (let i = 0; i < 10; i++) {
-        operations.push(
-          request
-            .post('/_lint')
-            .set(
-              'Authorization',
-              `Bearer ${createJWT(testRealm, 'john', ['read', 'write'])}`,
-            )
-            .set('X-HTTP-Method-Override', 'QUERY')
-            .set('Accept', 'application/json')
-            .send(testSource),
-        );
+        operations.push(lintOnce());
       }
 
       const results = await Promise.all(operations);
@@ -375,13 +392,29 @@ export class MyCard extends CardDef {
         );
       });
 
+      forceGc();
       const finalMemory = process.memoryUsage().heapUsed;
       const memoryIncrease = finalMemory - initialMemory;
+      const memoryIncreaseMb = memoryIncrease / 1024 / 1024;
+
+      // Always record the retained-growth number (and whether GC actually ran)
+      // so a CI failure — or a passing run drifting toward the bound — can be
+      // told apart from measurement noise without another CI cycle.
+      console.log(
+        `[lint-memory-test] initial=${(initialMemory / 1024 / 1024).toFixed(2)}MB ` +
+          `final=${(finalMemory / 1024 / 1024).toFixed(2)}MB ` +
+          `retainedGrowth=${memoryIncreaseMb.toFixed(2)}MB gcForced=${gcForced}`,
+      );
 
-      // Memory increase should be reasonable (less than 45MB for lint operations)
+      // The bound only means "retained memory" when a collection actually ran;
+      // with warm caches and a forced GC ten idempotent lint operations retain
+      // almost nothing, so 20MB is a generous leak guard. If GC could not be
+      // forced the delta still includes transient garbage, so fall back to the
+      // looser historical bound rather than flaking against the tight one.
+      const thresholdMb = gcForced ? 20 : 45;
       assert.ok(
-        memoryIncrease < 45 * 1024 * 1024,
-        `Memory increase should be under 45MB, got ${(memoryIncrease / 1024 / 1024).toFixed(2)}MB`,
+        memoryIncrease < thresholdMb * 1024 * 1024,
+        `Memory increase should be under ${thresholdMb}MB, got ${memoryIncreaseMb.toFixed(2)}MB (gcForced=${gcForced})`,
       );
     });