Skip to content
This repository has been archived by the owner on Mar 23, 2021. It is now read-only.

Close all handles during Jest teardown #2330

Merged
merged 6 commits into from
Mar 30, 2020
Merged

Close all handles during Jest teardown #2330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e4e4ab3
Add yalc files to .gitignore
luckysori Mar 28, 2020
b794b08
Replace timeout function with external dependency
luckysori Mar 30, 2020
6f24ff2
Configure Jasmine to report current test name
luckysori Mar 26, 2020
9f75317
Close all blockchain-related handles during Jest teardown
luckysori Mar 28, 2020
251c44f
Shut down logger during Jest teardown
luckysori Mar 28, 2020
3fb3ffb
Close log files after 5 seconds of inactivity
luckysori Mar 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,5 @@ TAGS
# Blockchain nodes artifacts
blockchain_nodes/bitcoin/
blockchain_nodes/parity/parity
.yalc
yalc.lock
1 change: 1 addition & 0 deletions api_tests/jest.config-dry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
};
1 change: 1 addition & 0 deletions api_tests/jest.config-e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
};
8 changes: 5 additions & 3 deletions api_tests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}'"
Expand All @@ -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",
Expand All @@ -43,16 +44,17 @@
"chai-json-schema": "^1.5.0",
"chai-string": "^1.5.0",
"chai-subset": "^1.6.0",
"comit-sdk": "^0.15.1",
"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",
Expand Down
11 changes: 6 additions & 5 deletions api_tests/src/actor_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

/*
Expand All @@ -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(
Expand All @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions api_tests/src/configure_jasmine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
jasmine.getEnv().addReporter({
specStarted: (result) =>
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get typings for this instead of using ts-ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can help me figure this one out.

I now realise that I'm using the jasmine namespace to set and read a value which wasn't there in the first place i.e. there is no type because I'm creating that "field".

Would it make more sense to save this to the global context of the e2e test suite?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to save this to the global context of the e2e test suite?

I guess not because this global context can be accessed by two tests running concurrently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just gonna leave this one as an optional follow-up PR. Had a look at merging namespaces to extend the one provided by jest with the type we need here, but there's probably a much easier approach that I'm missing!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess not because this global context can be accessed by two tests running concurrently.

The test environment provides a unique instance of global to every test so you can safely set things there!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, we can do this as a follow-up. I don't have time at the moment so you could write a short issue please? :)

(jasmine.currentTestName = result.description),
});
5 changes: 4 additions & 1 deletion api_tests/src/dry_test_environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 //
Expand Down Expand Up @@ -55,6 +55,7 @@ export default class DryTestEnvironment extends NodeEnvironment {
type: "pattern",
pattern: "%d %5.10p: %m",
},
timeout: 5000,
},
},
categories: {
Expand Down Expand Up @@ -83,6 +84,8 @@ export default class DryTestEnvironment extends NodeEnvironment {

async teardown() {
await super.teardown();

loggerShutdown();
}

private extractDocblockPragmas(
Expand Down
18 changes: 13 additions & 5 deletions api_tests/src/e2e_test_environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 //
Expand Down Expand Up @@ -73,6 +73,7 @@ export default class E2ETestEnvironment extends NodeEnvironment {
type: "pattern",
pattern: "%d %5.10p: %m",
},
timeout: 5000,
},
},
categories: {
Expand Down Expand Up @@ -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());
luckysori marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Expand Down
13 changes: 0 additions & 13 deletions api_tests/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions api_tests/src/wallets/bitcoin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,8 @@ export class BitcoinWallet implements Wallet {

return blockchainInfo.mediantime;
}

public async close(): Promise<void> {
return this.inner.close();
}
}
14 changes: 14 additions & 0 deletions api_tests/src/wallets/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions api_tests/src/wallets/lightning.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion api_tests/tests/dry/sanity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
5 changes: 0 additions & 5 deletions api_tests/types/smack-my-jasmine-up/index.d.ts
View file
Open in desktop

This file was deleted.

38 changes: 29 additions & 9 deletions api_tests/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1557,10 +1562,10 @@ combined-stream@^1.0.6, combined-stream@~1.0.6:
dependencies:
delayed-stream "~1.0.0"

comit-sdk@^0.15.1:
version "0.15.1"
resolved "https://registry.yarnpkg.com/comit-sdk/-/comit-sdk-0.15.1.tgz#2ccc262a9ba5ee86999284c773f34d659b967b8d"
integrity sha512-5oKJ1Ns4/9qei9AzzeeZOrb36Qb0yewpKQyvV/odCBQrMqbXHhcrGl0ArC6X5N7dED0IDEAI7J4G2qrVCsG1Cg==
comit-sdk@^0.15.2:
version "0.15.2"
resolved "https://registry.yarnpkg.com/comit-sdk/-/comit-sdk-0.15.2.tgz#affca33aed1cc18e7a3d39109a47041e3f0864e0"
integrity sha512-5HTRYRTR2Qpeiq29sTuTtPVkIF2CxEA7cku/8G3hHNp+EK2qicaV6bvJ9rhRitKY5EWSmWzBLOg+NzpCqvyyPg==
dependencies:
"@radar/lnrpc" "^0.9.1-beta"
axios "^0.19.0"
Expand Down Expand Up @@ -2781,6 +2786,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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down