wip - improve gc tests more

zeromq · Jun 16, 2024 · a1715e1 · a1715e1
1 parent 1f81c4a
commit a1715e1
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 144 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -79,17 +79,15 @@ jobs:
       ARCH: ${{ matrix.ARCH }}
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/checkout@v4
-
-      # - name: Cache
-      #   uses: actions/cache@v4
-      #   with:
-      #     path: |
-      #       ./node_modules/
-      #       ./build/
-      #     key: "cache-OS:${{ matrix.os }}-arch:${{ matrix.node_arch }}-ZMQ_DRAFT:${{ matrix.zmq_draft }}-Node:${{ matrix.node_version }}-${{ hashFiles('./package.json') }}"
-      #     restore-keys: |
-      #       "cache-OS:${{ matrix.os }}-arch:${{ matrix.node_arch }}-ZMQ_DRAFT:${{ matrix.zmq_draft }}-Node:${{ matrix.node_version }}-"
+      - name: Cache
+        uses: actions/cache@v4
+        with:
+          path: |
+            ./node_modules/
+            ./build/
+          key: "cache-OS:${{ matrix.os }}-arch:${{ matrix.node_arch }}-ZMQ_DRAFT:${{ matrix.zmq_draft }}-Node:${{ matrix.node_version }}-${{ hashFiles('./package.json') }}"
+          restore-keys: |
+            "cache-OS:${{ matrix.os }}-arch:${{ matrix.node_arch }}-ZMQ_DRAFT:${{ matrix.zmq_draft }}-Node:${{ matrix.node_version }}-"
 
       - name: Setup Cpp
         if: ${{ !matrix.docker }}
@@ -108,7 +106,6 @@ jobs:
       - name: Install Node
         if: ${{ !matrix.docker }}
         uses: actions/setup-node@v4
-        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node_version }}
           architecture: ${{ matrix.node_arch }}
@@ -155,7 +152,6 @@ jobs:
       - name: Test (Debug)
         if: ${{ !matrix.docker }}
         uses: nick-fields/retry@v3
-        uses: nick-fields/retry@v3
         with:
           timeout_minutes: 5
           max_attempts: 1
@@ -180,7 +176,6 @@ jobs:
       - name: Tests + GC Tests (Release)
         if: ${{ !matrix.docker }}
         uses: nick-fields/retry@v3
-        uses: nick-fields/retry@v3
         with:
           timeout_minutes: 5
           max_attempts: 1

diff --git a/test/.eslintrc b/test/.eslintrc
@@ -9,6 +9,7 @@
     "@typescript-eslint/no-use-before-define": "off",
     "@typescript-eslint/no-explicit-any": "off",
     "@typescript-eslint/no-unused-vars": "off",
+    "@typescript-eslint/strict-boolean-expressions": "off",
     "@typescript-eslint/explicit-function-return-type": "off",
     "no-await-in-loop": "off",
     "require-await": "off",

diff --git a/test/package.json b/test/package.json
diff --git a/test/pnpm-lock.yaml b/test/pnpm-lock.yaml
diff --git a/test/tsconfig.json b/test/tsconfig.json
@@ -1,4 +1,17 @@
 {
   "extends": "../tsconfig.json",
-  "include": ["**/*.ts"]
+  "include": [
+    "**/*.ts"
+  ],
+  "ts-node": {
+    "files": true
+  },
+  "compilerOptions": {
+    "types": [
+      "mocha"
+    ],
+    "lib": [
+      "ESNext"
+    ]
+  }
 }
diff --git a/test/unit/helpers.ts b/test/unit/helpers.ts
@@ -162,3 +162,43 @@ export async function captureEventsUntil(
 
   return events
 }
+
+// REAL typings for global.gc per
+// https://github.com/nodejs/node/blob/v20.0.0/deps/v8/src/extensions/gc-extension.cc
+interface GCFunction {
+  (options: {
+    execution?: "sync"
+    flavor?: "regular" | "last-resort"
+    type?: "major-snapshot" | "major" | "minor"
+    filename?: string
+  }): void
+  (options: {
+    execution?: "async"
+    flavor?: "regular" | "last-resort"
+    type?: "major-snapshot" | "major" | "minor"
+    filename?: string
+  }): Promise<void>
+  (options: {
+    execution?: "async" | "sync"
+    flavor?: "regular" | "last-resort"
+    type?: "major-snapshot" | "major" | "minor"
+    filename?: string
+  }): void | Promise<void>
+}
+
+export function getGcOrSkipTest(test: Mocha.Context) {
+  if (process.env.SKIP_GC_TESTS) {
+    test.skip()
+  }
+
+  const gc = globalThis.gc as undefined | GCFunction
+  if (typeof gc !== "function") {
+    throw new Error(
+      "Garbage collection is not exposed. It may be enabled by the node --expose-gc flag. To skip GC tests, set the environment variable `SKIP_GC_TESTS`",
+    )
+  }
+  // https://github.com/nodejs/node/blob/v20.0.0/deps/v8/src/extensions/gc-extension.h
+  // per docs, we we're using use case 2 (Test that certain objects indeed are reclaimed)
+  const asyncMajorGc = () => gc({type: "major", execution: "async"})
+  return asyncMajorGc
+}
diff --git a/test/unit/socket-close-test.ts b/test/unit/socket-close-test.ts
@@ -2,7 +2,7 @@
 import * as zmq from "../../src"
 
 import {assert} from "chai"
-import {testProtos, uniqAddress} from "./helpers"
+import {testProtos, uniqAddress, getGcOrSkipTest} from "./helpers"
 import {isFullError} from "../../src/errors"
 
 for (const proto of testProtos("tcp", "ipc", "inproc")) {
@@ -107,9 +107,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
       })
 
       it("should release reference to context", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = getGcOrSkipTest(this)
         this.slow(200)
 
         let weakRef: undefined | WeakRef<any>
@@ -125,10 +123,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
         }
 
         await task()
-        global.gc?.()
-        await new Promise(resolve => {
-          setTimeout(resolve, 5)
-        })
+        await gc()
 
         assert.isDefined(weakRef)
         assert.isUndefined(weakRef!.deref())
@@ -137,9 +132,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
 
     describe("in gc finalizer", function () {
       it("should release reference to context", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = getGcOrSkipTest(this)
         if (process.env.SKIP_GC_FINALIZER_TESTS) {
           this.skip()
         }
@@ -152,14 +145,12 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
           new zmq.Dealer({context, linger: 0})
           weakRef = new WeakRef(context)
           context = undefined
-          global.gc?.()
+          await gc()
         }
 
         await task()
-        global.gc?.()
-        await new Promise(resolve => {
-          setTimeout(resolve, 5)
-        })
+        await gc()
+
         assert.isDefined(weakRef)
         assert.isUndefined(weakRef!.deref())
       })

diff --git a/test/unit/socket-send-receive-test.ts b/test/unit/socket-send-receive-test.ts
@@ -2,7 +2,7 @@
 import * as zmq from "../../src"
 
 import {assert} from "chai"
-import {testProtos, uniqAddress} from "./helpers"
+import {testProtos, uniqAddress, getGcOrSkipTest} from "./helpers"
 import {isFullError} from "../../src/errors"
 
 for (const proto of testProtos("tcp", "ipc", "inproc")) {
@@ -91,9 +91,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
       })
 
       it("should copy and release small buffers", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = getGcOrSkipTest(this)
         let weakRef: undefined | WeakRef<any>
         sockA.connect(uniqAddress(proto))
         const send = async (size: number) => {
@@ -103,18 +101,13 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
         }
 
         await send(16)
-        global.gc?.()
-        await new Promise(resolve => {
-          setTimeout(resolve, 5)
-        })
+        await gc()
         assert.isDefined(weakRef)
         assert.isUndefined(weakRef!.deref())
       })
 
       it("should retain large buffers", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = getGcOrSkipTest(this)
         let weakRef: undefined | WeakRef<any>
 
         sockA.connect(uniqAddress(proto))
@@ -125,12 +118,9 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
         }
 
         await send(1025)
-        global.gc?.()
-        await new Promise(resolve => {
-          setTimeout(resolve, 5)
-        })
+        await gc()
         assert.isDefined(weakRef)
-        assert.isUndefined(weakRef!.deref())
+        assert.isDefined(weakRef.deref())
       })
     })
 
@@ -338,13 +328,11 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
       })
 
       it("should release buffers", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = await getGcOrSkipTest(this)
+
         const weakRefs: WeakRef<any>[] = []
 
         const n = 10
-        let released = 0
 
         const send = async (size: number) => {
           for (let i = 0; i < n; i++) {
@@ -365,7 +353,8 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
 
         /* Repeated GC to allow inproc messages from being collected. */
         for (let i = 0; i < 5; i++) {
-          global.gc?.()
+          await gc()
+
           await new Promise(resolve => {
             setTimeout(resolve, 2)
           })
@@ -377,14 +366,11 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
       })
 
       it("should release buffers after echo", async function () {
-        if (process.env.SKIP_GC_TESTS) {
-          this.skip()
-        }
+        const gc = getGcOrSkipTest(this)
 
         const weakRefs: WeakRef<any>[] = []
 
         const n = 10
-        let released = 0
 
         const echo = async () => {
           for (let i = 0; i < n; i++) {
@@ -412,10 +398,10 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
 
         /* Repeated GC to allow inproc messages from being collected. */
         for (let i = 0; i < 5; i++) {
-          global.gc?.()
           await new Promise(resolve => {
             setTimeout(resolve, 2)
           })
+          await gc()
         }
 
         assert.lengthOf(weakRefs, n * 3)

diff --git a/tsconfig.json b/tsconfig.json
@@ -6,7 +6,6 @@
     "module": "commonjs",
     "types": [
       "node",
-      "mocha"
     ],
     "strictPropertyInitialization": false, // TODO
     "strict": true,
@@ -21,7 +20,7 @@
     "sourceMap": true,
     "esModuleInterop": true,
     "lib": [
-      "ES2020",
+      "ES2022",
       "dom"
     ]
   }