From 62aba0e3d6de2daf8e5c4246eb75b97e3147c411 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sun, 1 May 2022 11:21:46 -0400 Subject: [PATCH 1/9] environment: cleanup connect or start logic --- packages/environment/develop.js | 39 ++++++++++++++------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/packages/environment/develop.js b/packages/environment/develop.js index ef3cff403c1..dc24c7ea3f7 100644 --- a/packages/environment/develop.js +++ b/packages/environment/develop.js @@ -4,7 +4,7 @@ const { spawn } = require("child_process"); const debug = require("debug"); const Develop = { - start: async function(ipcNetwork, options = {}) { + start: async function (ipcNetwork, options = {}) { let chainPath; // The path to the dev env process depends on whether or not @@ -39,7 +39,7 @@ const Develop = { }); }, - connect: function(options) { + connect: function (options) { const debugServer = debug("develop:ipc:server"); const debugClient = debug("develop:ipc:client"); const debugRPC = debug("develop:ganache"); @@ -69,7 +69,7 @@ const Develop = { if (options.log) { debugRPC.enabled = true; - loggers.ganache = function() { + loggers.ganache = function () { // HACK-y: replace `{}` that is getting logged instead of "" var args = Array.prototype.slice.call(arguments); if ( @@ -88,21 +88,21 @@ const Develop = { ipc.config.maxRetries = 0; } - var disconnect = function() { + var disconnect = function () { ipc.disconnect(ipcNetwork); }; return new Promise((resolve, reject) => { - ipc.connectTo(ipcNetwork, connectPath, function() { - ipc.of[ipcNetwork].on("destroy", function() { + ipc.connectTo(ipcNetwork, connectPath, function () { + ipc.of[ipcNetwork].on("destroy", function () { reject(new Error("IPC connection destroyed")); }); - ipc.of[ipcNetwork].on("truffle.ready", function() { + ipc.of[ipcNetwork].on("truffle.ready", function () { resolve(disconnect); }); - Object.keys(loggers).forEach(function(key) { + Object.keys(loggers).forEach(function (key) { var log = loggers[key]; if (log) { var message = `truffle.${key}.log`; @@ -113,30 +113,23 @@ const Develop = { }); }, - connectOrStart: async function(options, ganacheOptions) { + connectOrStart: async function (options, ganacheOptions) { options.retry = false; const ipcNetwork = options.network || "develop"; - let connectedAlready = false; + let started = false; + let disconnect; try { - const disconnect = await this.connect(options); - connectedAlready = true; - return { - started: false, - disconnect - }; + disconnect = await this.connect(options); } catch (_error) { await this.start(ipcNetwork, ganacheOptions); options.retry = true; - const disconnect = await this.connect(options); - if (connectedAlready) return; - connectedAlready = true; - return { - started: true, - disconnect - }; + disconnect = await this.connect(options); + started = true; + } finally { + return { disconnect, started }; } } }; From f360e1fedf5352f21d30e1934ffcf5fabcb5e8a2 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 28 Jan 2022 01:32:53 -0500 Subject: [PATCH 2/9] environment: update ganache.server.close This function [1] changed and returns a promise and does not accept a callback. 1: https://github.com/trufflesuite/ganache/blob/d5c3bcbcf2198d2664bd9bfafd331d80994afc7f/src/packages/core/src/server.ts#L272 --- packages/environment/chain.js | 11 +++++------ packages/environment/develop.js | 5 ++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/environment/chain.js b/packages/environment/chain.js index a43b72bd56d..e8427a08a42 100644 --- a/packages/environment/chain.js +++ b/packages/environment/chain.js @@ -227,14 +227,13 @@ class GanacheMixin { // cleanup Ganache process on exit exit(_supervisor) { - this.ganache.close(err => { - if (err) { + this.ganache + .close() + .then(() => process.exit()) + .catch(err => { console.error(err.stack || err); process.exit(1); - } else { - process.exit(); - } - }); + }); } } diff --git a/packages/environment/develop.js b/packages/environment/develop.js index dc24c7ea3f7..943f8a6f44a 100644 --- a/packages/environment/develop.js +++ b/packages/environment/develop.js @@ -129,7 +129,10 @@ const Develop = { disconnect = await this.connect(options); started = true; } finally { - return { disconnect, started }; + return { + disconnect, + started + }; } } }; From 6c9d67bc9a31af92b30ad000a07898d89f72b93b Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 28 Jan 2022 14:29:33 -0500 Subject: [PATCH 3/9] environment: add connectOrStart tests Test to validate code path to connect to an established Ganache server, or no Ganache server. --- .../environment/test/connectOrStartTest.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 packages/environment/test/connectOrStartTest.js diff --git a/packages/environment/test/connectOrStartTest.js b/packages/environment/test/connectOrStartTest.js new file mode 100644 index 00000000000..0f24a030950 --- /dev/null +++ b/packages/environment/test/connectOrStartTest.js @@ -0,0 +1,33 @@ +const assert = require("chai").assert; +const Develop = require("../develop"); + +describe("connectOrStart test network", async function () { + const ipcOptions = { network: "test" }; + const ganacheOptions = { + host: "127.0.0.1", + network_id: 666, + port: 6969 + }; + + it("starts Ganache when no Ganache instance is running", async function () { + const connection = await Develop.connectOrStart(ipcOptions, ganacheOptions); + assert.isTrue(connection.started); + assert.isFunction(connection.disconnect); + connection.disconnect(); + }); + + it("connects to an established ganache instance", async function () { + //establish a connection + const { disconnect: connectionOneDisconnect } = + await Develop.connectOrStart(ipcOptions, ganacheOptions); + + //invoke the method again + const { disconnect: connectionTwoDisconnect, started } = + await Develop.connectOrStart(ipcOptions, ganacheOptions); + assert.isFalse(started); + + //cleanup + await connectionOneDisconnect(); + await connectionTwoDisconnect(); + }); +}); From 17c0108771116a2d8f84634b767b3d2620e87d54 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 28 Jan 2022 18:07:46 -0500 Subject: [PATCH 4/9] environment: use finally for cleanup --- .../environment/test/connectOrStartTest.js | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/packages/environment/test/connectOrStartTest.js b/packages/environment/test/connectOrStartTest.js index 0f24a030950..a153bbaad46 100644 --- a/packages/environment/test/connectOrStartTest.js +++ b/packages/environment/test/connectOrStartTest.js @@ -10,24 +10,45 @@ describe("connectOrStart test network", async function () { }; it("starts Ganache when no Ganache instance is running", async function () { - const connection = await Develop.connectOrStart(ipcOptions, ganacheOptions); - assert.isTrue(connection.started); - assert.isFunction(connection.disconnect); - connection.disconnect(); + let connection; + try { + connection = await Develop.connectOrStart(ipcOptions, ganacheOptions); + assert.isTrue(connection.started, "A new server did not spin up"); + assert.isFunction(connection.disconnect, "Disconnect is not a function"); + } catch (error) { + console.log(error); + assert.fail("Develop.connectOrStart should not have thrown!"); + } finally { + if (connection) { + return connection.disconnect(); + } + } }); it("connects to an established ganache instance", async function () { - //establish a connection - const { disconnect: connectionOneDisconnect } = - await Develop.connectOrStart(ipcOptions, ganacheOptions); + let connectionOneDisconnect, connectionTwoDisconnect; - //invoke the method again - const { disconnect: connectionTwoDisconnect, started } = - await Develop.connectOrStart(ipcOptions, ganacheOptions); - assert.isFalse(started); + try { + //establish a connection + connectionOneDisconnect = ( + await Develop.connectOrStart(ipcOptions, ganacheOptions) + ).disconnect; - //cleanup - await connectionOneDisconnect(); - await connectionTwoDisconnect(); + //invoke the method again + const result = await Develop.connectOrStart(ipcOptions, ganacheOptions); + connectionTwoDisconnect = result.disconnect; + assert.isFalse(result.started); + } catch (error) { + console.log(error); + assert.fail("Develop.connectOrStart should not have thrown!"); + } finally { + //cleanup + if (connectionOneDisconnect) { + connectionOneDisconnect(); + } + if (connectionTwoDisconnect) { + connectionTwoDisconnect(); + } + } }); }); From 272929465295339d693004e34acc0befed38f454 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sat, 29 Jan 2022 12:06:56 -0500 Subject: [PATCH 5/9] environment: allow exceptions to bubble up - remove catch - add messages to assertions --- .../environment/test/connectOrStartTest.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/environment/test/connectOrStartTest.js b/packages/environment/test/connectOrStartTest.js index a153bbaad46..549204f22de 100644 --- a/packages/environment/test/connectOrStartTest.js +++ b/packages/environment/test/connectOrStartTest.js @@ -13,19 +13,16 @@ describe("connectOrStart test network", async function () { let connection; try { connection = await Develop.connectOrStart(ipcOptions, ganacheOptions); - assert.isTrue(connection.started, "A new server did not spin up"); - assert.isFunction(connection.disconnect, "Disconnect is not a function"); - } catch (error) { - console.log(error); - assert.fail("Develop.connectOrStart should not have thrown!"); + assert.isTrue(connection.started, "A new Ganache server did not spin up"); + assert.isFunction(connection.disconnect, "disconnect is not a function"); } finally { if (connection) { - return connection.disconnect(); + connection.disconnect(); } } }); - it("connects to an established ganache instance", async function () { + it("connects to an established Ganache instance", async function () { let connectionOneDisconnect, connectionTwoDisconnect; try { @@ -37,10 +34,10 @@ describe("connectOrStart test network", async function () { //invoke the method again const result = await Develop.connectOrStart(ipcOptions, ganacheOptions); connectionTwoDisconnect = result.disconnect; - assert.isFalse(result.started); - } catch (error) { - console.log(error); - assert.fail("Develop.connectOrStart should not have thrown!"); + assert.isFalse( + result.started, + "Should have connected to established Ganache server" + ); } finally { //cleanup if (connectionOneDisconnect) { From 8b088178d02f60a74874e1cbca2dc666616deed8 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Fri, 29 Apr 2022 05:34:28 -0400 Subject: [PATCH 6/9] environment: give chain process time to clean-up set instead of invoking , which forces the process to exit as quickly as possible even if there are still asynchronous operations pending that have not yet completed fully, including I/O operations to process.stdout and process.stderr. --- packages/environment/chain.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/environment/chain.js b/packages/environment/chain.js index e8427a08a42..587cd5aa9bf 100644 --- a/packages/environment/chain.js +++ b/packages/environment/chain.js @@ -229,10 +229,10 @@ class GanacheMixin { exit(_supervisor) { this.ganache .close() - .then(() => process.exit()) + .then(() => (process.exitCode = 0)) .catch(err => { console.error(err.stack || err); - process.exit(1); + process.exitCode = 1; }); } } From f57ba33310f7e3bfca51d9f5b25608eb846eb41a Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sat, 30 Apr 2022 23:16:39 -0400 Subject: [PATCH 7/9] environment: add mocha eslint definitions --- packages/environment/.eslintrc.json | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 packages/environment/.eslintrc.json diff --git a/packages/environment/.eslintrc.json b/packages/environment/.eslintrc.json new file mode 100644 index 00000000000..e0748c67bfe --- /dev/null +++ b/packages/environment/.eslintrc.json @@ -0,0 +1,3 @@ +{ + "extends": ["../../.eslintrc.package.json"] +} From 076d0094e2380634fedf38e9e5bcae78a006a2da Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sat, 30 Apr 2022 23:16:48 -0400 Subject: [PATCH 8/9] environment: rework test setup and teardown --- .../environment/test/connectOrStartTest.js | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/environment/test/connectOrStartTest.js b/packages/environment/test/connectOrStartTest.js index 549204f22de..e8b9d5de678 100644 --- a/packages/environment/test/connectOrStartTest.js +++ b/packages/environment/test/connectOrStartTest.js @@ -23,29 +23,35 @@ describe("connectOrStart test network", async function () { }); it("connects to an established Ganache instance", async function () { - let connectionOneDisconnect, connectionTwoDisconnect; + let connectionOneDisconnect, connectionTwo; + let spawnedGanache; try { - //establish a connection - connectionOneDisconnect = ( - await Develop.connectOrStart(ipcOptions, ganacheOptions) - ).disconnect; + //Establish IPC Ganache service + spawnedGanache = await Develop.start(ipcOptions.network, ganacheOptions); + connectionOneDisconnect = await Develop.connect({ + ...ipcOptions, + retry: true + }); - //invoke the method again - const result = await Develop.connectOrStart(ipcOptions, ganacheOptions); - connectionTwoDisconnect = result.disconnect; - assert.isFalse( - result.started, - "Should have connected to established Ganache server" - ); + //Test + connectionTwo = await Develop.connectOrStart(ipcOptions, ganacheOptions); + + //Validate + assert.isFalse(connectionTwo.started); } finally { - //cleanup + //Cleanup IPC2 + if (connectionTwo.disconnect) { + connectionTwo.disconnect(); + } + //Cleanup IPC1 if (connectionOneDisconnect) { connectionOneDisconnect(); } - if (connectionTwoDisconnect) { - connectionTwoDisconnect(); - } + //Signal Ganache Process to exit + process.kill(spawnedGanache.pid, "SIGHUP"); + //Resolve on confirmation of process exit + await new Promise(resolve => spawnedGanache.on("exit", () => resolve())); } }); }); From 89f59adb2c9d44d8416946c01c7bff5c58887dfa Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sun, 1 May 2022 10:35:12 -0400 Subject: [PATCH 9/9] environment: refactor and comment a test --- packages/environment/test/develop.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/environment/test/develop.js b/packages/environment/test/develop.js index 37234302d14..53be8c6b1e3 100644 --- a/packages/environment/test/develop.js +++ b/packages/environment/test/develop.js @@ -29,17 +29,15 @@ describe("Environment develop", function () { gas: expectedNetwork.gas } }; - sinon.stub(Environment, "detect"); }); - afterEach("Restore Environment detect", async function () { - Environment.detect.restore(); - }); - - it("Environment.develop overwrites the network_id of the network", async function() { + it("Environment.develop overwrites the network_id of the network", async function () { + //Stub detect in order to prevent its executing during this test + //which is invoked during `Environment.develop(..)` + sinon.stub(Environment, "detect"); await Environment.develop(config, testOptions); - const mutatedNetwork = config.networks[config.network]; + const mutatedNetwork = config.networks[config.network]; assert.equal( mutatedNetwork.port, expectedNetwork.port, @@ -55,6 +53,8 @@ describe("Environment develop", function () { expectedNetwork.gas, "The gas of the network should be unchanged." ); - }); + //Restore `detect` functionality + Environment.detect.restore(); + }); }).timeout(10000);