Merge #2330

2330: Close all handles during Jest teardown r=bonomat a=luckysori _Waiting for comit-network/comit-js-sdk#176 to be merged and for those changes to be included in a release of `comit-js-sdk` before we can update the last commit in this PR to use that release instead of a local dependency created via [`yalc`](https://github.com/whitecolor/yalc) (which seems to work **much** better than `yarn link`)._ Fixes #2211. 1. Turns out Jest will hang until every timeout is cleared, so racing against a timeout that is only cleared after it resolves was not good. 2. The proper teardown of the e2e test environment is fully achieved with 3698f3a, after a lot of debugging. Unfortunately, Jest recommends this ``` Consider running Jest with `--detectOpenHandles` to troubleshoot this issue ``` when tests hang after passing, but it just [doesn't seem to do anything](jestjs/jest#9473). I tried using [`leaked-handles`](https://github.com/Raynos/leaked-handles) without success, but finally found [`wtfnode`](https://github.com/myndzi/wtfnode), which identified `bcoin` as the culprit. --- My OCD was definitely triggered with this one, as a different error got me to investigate how to get rid of `smack-my-jasmine-up`. Turns out removing it did not fix that other error, but I still see it as a win, since depending on such an obscure library is bad. --- And another bug squashed! No more ``` MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGHUP listeners added to [process]. Use emitter.setMaxListeners() to increase limit ``` errors, by combining cde8e0c and 127a4a2: - shutting down the logger during teardown. - closing log files after 5 seconds of inactivity, to prevent us from reaching the limit of listeners. I believe log files can be reopened if something else needs to be logged. Co-authored-by: Lucas Soriano del Pino <l.soriano.del.pino@gmail.com>
comit-network · Mar 30, 2020 · 23dda65 · 23dda65
2 parents 12abf2c + 3fb3ffb
commit 23dda65
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 37 deletions.
diff --git a/.gitignore b/.gitignore
@@ -61,3 +61,5 @@ TAGS
 # Blockchain nodes artifacts
 blockchain_nodes/bitcoin/
 blockchain_nodes/parity/parity
+.yalc
+yalc.lock
diff --git a/api_tests/jest.config-dry.js b/api_tests/jest.config-dry.js
@@ -8,4 +8,5 @@ module.exports = {
     moduleFileExtensions: ["ts", "js", "json", "node"],
     testEnvironment: "<rootDir>/dist/src/dry_test_environment",
     testTimeout: 63000,
+    setupFilesAfterEnv: ["<rootDir>/src/configure_jasmine.ts"],
 };
diff --git a/api_tests/jest.config-e2e.js b/api_tests/jest.config-e2e.js
@@ -8,4 +8,5 @@ module.exports = {
     moduleFileExtensions: ["ts", "js", "json", "node"],
     testEnvironment: "<rootDir>/dist/src/e2e_test_environment",
     testTimeout: 63000,
+    setupFilesAfterEnv: ["<rootDir>/src/configure_jasmine.ts"],
 };
diff --git a/api_tests/package.json b/api_tests/package.json
@@ -7,7 +7,7 @@
         "check": "tsc && prettier --check '**/*.{ts,json,yml}' && tslint --project .",
         "pretest": "cargo build --bin cnd && tsc",
         "dry": "jest --config jest.config-dry.js --maxWorkers=4",
-        "e2e": "yarn jest --config jest.config-e2e.js --runInBand --forceExit --bail",
+        "e2e": "jest --config jest.config-e2e.js --runInBand --forceExit --bail",
         "test": "yarn dry && yarn e2e",
         "ci": "tsc && yarn dry && yarn e2e",
         "fix": "tslint --project . --fix && prettier --write '**/*.{ts,js,json,yml}'"
@@ -24,6 +24,7 @@
         "@types/chai-as-promised": "^7.1.2",
         "@types/chai-json-schema": "^1.4.5",
         "@types/chai-subset": "^1.3.3",
+        "@types/jasmine": "^3.5.10",
         "@types/jest": "^25.1.4",
         "@types/log4js": "^2.3.5",
         "@types/node": "^13.9",
@@ -46,13 +47,14 @@
         "comit-sdk": "^0.15.2",
         "ethers": "^4.0.46",
         "get-port": "^5.1.1",
+        "jasmine": "^3.5.0",
         "jest": "^25.2.3",
         "js-sha256": "^0.9.0",
         "log4js": "^6.1.2",
+        "p-timeout": "^3.2.0",
         "prettier": "^2.0.2",
         "rimraf": "^3.0.2",
         "satoshi-bitcoin": "^1.0.4",
-        "smack-my-jasmine-up": "^0.0.3",
         "tail": "^2.0.3",
         "temp-write": "^4.0.0",
         "tmp": "^0.1.0",

diff --git a/api_tests/src/actor_test.ts b/api_tests/src/actor_test.ts
@@ -1,7 +1,6 @@
 import { Actors } from "./actors";
 import { createActors } from "./create_actors";
-import JasmineSmacker from "smack-my-jasmine-up";
-import { timeout } from "./utils";
+import pTimeout from "p-timeout";
 import ProvidesCallback = jest.ProvidesCallback;
 
 /*
@@ -13,7 +12,8 @@ function nActorTest(
     testFn: (actors: Actors) => Promise<void>
 ): ProvidesCallback {
     return async (done) => {
-        const name = JasmineSmacker.getCurrentTestName();
+        // @ts-ignore
+        const name = jasmine.currentTestName;
         if (!name.match(/[A-z0-9\-]+/)) {
             // We use the test name as a file name for the log and hence need to restrict it.
             throw new Error(
@@ -24,15 +24,16 @@ function nActorTest(
         const actors = await createActors(name, actorNames);
 
         try {
-            await timeout(60_000, testFn(actors));
+            await pTimeout(testFn(actors), 60_000);
         } catch (e) {
             for (const actorName of actorNames) {
                 await actors.getActorByName(actorName).dumpState();
             }
             throw e;
         } finally {
             for (const actorName of actorNames) {
-                await actors.getActorByName(actorName).stop();
+                const actor = actors.getActorByName(actorName);
+                await Promise.all([actor.stop(), actor.wallets.close()]);
             }
         }
         done();

diff --git a/api_tests/src/configure_jasmine.ts b/api_tests/src/configure_jasmine.ts
@@ -0,0 +1,5 @@
+jasmine.getEnv().addReporter({
+    specStarted: (result) =>
+        // @ts-ignore
+        (jasmine.currentTestName = result.description),
+});
diff --git a/api_tests/src/dry_test_environment.ts b/api_tests/src/dry_test_environment.ts
@@ -3,7 +3,7 @@ import { execAsync, HarnessGlobal, mkdirAsync, rimrafAsync } from "./utils";
 import NodeEnvironment from "jest-environment-node";
 import { Mutex } from "async-mutex";
 import path from "path";
-import { configure } from "log4js";
+import { configure, shutdown as loggerShutdown } from "log4js";
 
 // ************************ //
 // Setting global variables //
@@ -55,6 +55,7 @@ export default class DryTestEnvironment extends NodeEnvironment {
                         type: "pattern",
                         pattern: "%d %5.10p: %m",
                     },
+                    timeout: 5000,
                 },
             },
             categories: {
@@ -83,6 +84,8 @@ export default class DryTestEnvironment extends NodeEnvironment {
 
     async teardown() {
         await super.teardown();
+
+        loggerShutdown();
     }
 
     private extractDocblockPragmas(

diff --git a/api_tests/src/e2e_test_environment.ts b/api_tests/src/e2e_test_environment.ts
@@ -13,7 +13,7 @@ import EthereumLedger from "./ledgers/ethereum";
 import LightningLedger from "./ledgers/lightning";
 import { ParityInstance } from "./ledgers/parity_instance";
 import { LndInstance } from "./ledgers/lnd_instance";
-import { configure, Logger } from "log4js";
+import { configure, Logger, shutdown as loggerShutdown } from "log4js";
 
 // ************************ //
 // Setting global variables //
@@ -73,6 +73,7 @@ export default class E2ETestEnvironment extends NodeEnvironment {
                         type: "pattern",
                         pattern: "%d %5.10p: %m",
                     },
+                    timeout: 5000,
                 },
             },
             categories: {
@@ -251,26 +252,33 @@ export default class E2ETestEnvironment extends NodeEnvironment {
         this.logger.info("Tearing down test environment");
 
         await this.cleanupAll();
+
+        loggerShutdown();
+
         this.logger.info("Tearing down complete");
     }
 
     async cleanupAll() {
         const tasks = [];
 
         if (this.bitcoinLedger) {
-            tasks.push(this.bitcoinLedger.stop);
+            tasks.push(this.bitcoinLedger.stop());
         }
 
         if (this.ethereumLedger) {
-            tasks.push(this.ethereumLedger.stop);
+            tasks.push(this.ethereumLedger.stop());
         }
 
         if (this.aliceLightning) {
-            tasks.push(this.aliceLightning.stop);
+            tasks.push(this.aliceLightning.stop());
         }
 
         if (this.bobLightning) {
-            tasks.push(this.bobLightning.stop);
+            tasks.push(this.bobLightning.stop());
+        }
+
+        for (const [, wallet] of Object.entries(this.global.lndWallets)) {
+            tasks.push(wallet.close());
         }
 
         await Promise.all(tasks);

diff --git a/api_tests/src/utils.ts b/api_tests/src/utils.ts
@@ -46,19 +46,6 @@ export async function sleep(time: number) {
     });
 }
 
-export async function timeout<T>(ms: number, promise: Promise<T>): Promise<T> {
-    // Create a promise that rejects in <ms> milliseconds
-    const timeout = new Promise<T>((_, reject) => {
-        const id = setTimeout(() => {
-            clearTimeout(id);
-            reject(new Error(`timed out after ${ms}ms`));
-        }, ms);
-    });
-
-    // Returns a race between our timeout and the passed in promise
-    return Promise.race([promise, timeout]);
-}
-
 export const DEFAULT_ALPHA = {
     ledger: {
         name: "bitcoin",

diff --git a/api_tests/src/wallets/bitcoin.ts b/api_tests/src/wallets/bitcoin.ts
@@ -98,4 +98,8 @@ export class BitcoinWallet implements Wallet {
 
         return blockchainInfo.mediantime;
     }
+
+    public async close(): Promise<void> {
+        return this.inner.close();
+    }
 }
diff --git a/api_tests/src/wallets/index.ts b/api_tests/src/wallets/index.ts
@@ -83,6 +83,20 @@ export class Wallets {
                 }
         }
     }
+
+    public async close(): Promise<void[]> {
+        const tasks = [];
+
+        if (this.wallets.lightning) {
+            tasks.push(this.wallets.lightning.close());
+        }
+
+        if (this.wallets.bitcoin) {
+            tasks.push(this.wallets.bitcoin.close());
+        }
+
+        return Promise.all(tasks);
+    }
 }
 
 export async function pollUntilMinted(

diff --git a/api_tests/src/wallets/lightning.ts b/api_tests/src/wallets/lightning.ts
@@ -172,4 +172,8 @@ export class LightningWallet implements Wallet {
         await sleep(500);
         return this.pollUntilChannelIsOpen(outpoint);
     }
+
+    public async close(): Promise<void> {
+        return this.bitcoinWallet.close();
+    }
 }
diff --git a/api_tests/tests/dry/sanity.ts b/api_tests/tests/dry/sanity.ts
@@ -11,7 +11,7 @@ import { Entity, Link } from "comit-sdk";
 // Sanity tests                                 //
 // ******************************************** //
 
-describe("Sanity - peers using IP", () => {
+describe("Sanity", () => {
     it(
         "invalid-swap-yields-404",
         oneActorTest(async ({ alice }) => {

diff --git a/api_tests/types/smack-my-jasmine-up/index.d.ts b/api_tests/types/smack-my-jasmine-up/index.d.ts
diff --git a/api_tests/yarn.lock b/api_tests/yarn.lock
@@ -544,6 +544,11 @@
     "@types/istanbul-lib-coverage" "*"
     "@types/istanbul-lib-report" "*"
 
+"@types/jasmine@^3.5.10":
+  version "3.5.10"
+  resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-3.5.10.tgz#a1a41012012b5da9d4b205ba9eba58f6cce2ab7b"
+  integrity sha512-3F8qpwBAiVc5+HPJeXJpbrl+XjawGmciN5LgiO7Gv1pl1RHtjoMNqZpqEksaPJW05ViKe8snYInRs6xB25Xdew==
+
 "@types/jest@^25.1.4":
   version "25.1.4"
   resolved "https://registry.yarnpkg.com/@types/jest/-/jest-25.1.4.tgz#9e9f1e59dda86d3fd56afce71d1ea1b331f6f760"
@@ -2776,6 +2781,19 @@ istanbul-reports@^3.0.0:
     html-escaper "^2.0.0"
     istanbul-lib-report "^3.0.0"
 
+jasmine-core@~3.5.0:
+  version "3.5.0"
+  resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-3.5.0.tgz#132c23e645af96d85c8bca13c8758b18429fc1e4"
+  integrity sha512-nCeAiw37MIMA9w9IXso7bRaLl+c/ef3wnxsoSAlYrzS+Ot0zTG6nU8G/cIfGkqpkjX2wNaIW9RFG0TwIFnG6bA==
+
+jasmine@^3.5.0:
+  version "3.5.0"
+  resolved "https://registry.yarnpkg.com/jasmine/-/jasmine-3.5.0.tgz#7101eabfd043a1fc82ac24e0ab6ec56081357f9e"
+  integrity sha512-DYypSryORqzsGoMazemIHUfMkXM7I7easFaxAvNM3Mr6Xz3Fy36TupTrAOxZWN8MVKEU5xECv22J4tUQf3uBzQ==
+  dependencies:
+    glob "^7.1.4"
+    jasmine-core "~3.5.0"
+
 jest-changed-files@^25.2.3:
   version "25.2.3"
   resolved "https://registry.yarnpkg.com/jest-changed-files/-/jest-changed-files-25.2.3.tgz#ad19deef9e47ba37efb432d2c9a67dfd97cc78af"
@@ -3915,6 +3933,13 @@ p-timeout@^2.0.1:
   dependencies:
     p-finally "^1.0.0"
 
+p-timeout@^3.2.0:
+  version "3.2.0"
+  resolved "https://registry.yarnpkg.com/p-timeout/-/p-timeout-3.2.0.tgz#c7e17abc971d2a7962ef83626b35d635acf23dfe"
+  integrity sha512-rhIwUycgwwKcP9yTOOFK/AKsAopjjCakVqLHePO3CC6Mir1Z99xT+R63jZxAT5lFZLa2inS5h+ZS2GvR99/FBg==
+  dependencies:
+    p-finally "^1.0.0"
+
 p-try@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3"
@@ -4540,11 +4565,6 @@ slash@^3.0.0:
   resolved "https://registry.yarnpkg.com/slash/-/slash-3.0.0.tgz#6539be870c165adbd5240220dbe361f1bc4d4634"
   integrity sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==
 
-smack-my-jasmine-up@^0.0.3:
-  version "0.0.3"
-  resolved "https://registry.yarnpkg.com/smack-my-jasmine-up/-/smack-my-jasmine-up-0.0.3.tgz#44c9420220f7bd90b7ffe329b343828eb3c05972"
-  integrity sha512-DLaxxk5L3vrnjgqRwgUsFtmw4CvjnOmaXdxHp89rKTxt03erWQ0xgE0ZKZtllv+3BH3xljF5fR5pHIe27OLRbg==
-
 snapdragon-node@^2.0.1:
   version "2.1.1"
   resolved "https://registry.yarnpkg.com/snapdragon-node/-/snapdragon-node-2.1.1.tgz#6c175f86ff14bdb0724563e8f3c1b021a286853b"