From 1fd025802b65e3fe0841cd0375d6f9ee44ae68e4 Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Fri, 15 Nov 2024 16:34:09 -0500 Subject: [PATCH 1/6] Spawn call was missing shell true option --- workbench/src/main/setupAddRemovePlugin.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workbench/src/main/setupAddRemovePlugin.js b/workbench/src/main/setupAddRemovePlugin.js index 34311a74d..2676881df 100644 --- a/workbench/src/main/setupAddRemovePlugin.js +++ b/workbench/src/main/setupAddRemovePlugin.js @@ -27,7 +27,8 @@ const logger = getLogger(__filename.split('/').slice(-1)[0]); */ function spawnWithLogging(cmd, args, options) { logger.info(cmd, args); - const cmdProcess = spawn(cmd, args, { ...options, windowsHide: true }); + const cmdProcess = spawn( + cmd, args, { ...options, shell: true, windowsHide: true }); if (cmdProcess.stdout) { cmdProcess.stderr.on('data', (data) => logger.info(data.toString())); cmdProcess.stdout.on('data', (data) => logger.info(data.toString())); From eeaa41cfceb21359d6fea7a7f3826e958f6ceedb Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Wed, 20 Nov 2024 09:06:22 -0500 Subject: [PATCH 2/6] Update how investUsageLogger gets PORT --- workbench/src/main/investUsageLogger.js | 8 ++++---- workbench/src/main/setupInvestHandlers.js | 7 +++++-- workbench/tests/main/main.test.js | 7 ++++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/workbench/src/main/investUsageLogger.js b/workbench/src/main/investUsageLogger.js index 835ec2299..e34daec39 100644 --- a/workbench/src/main/investUsageLogger.js +++ b/workbench/src/main/investUsageLogger.js @@ -13,9 +13,9 @@ const PREFIX = 'api'; export default function investUsageLogger() { const sessionId = crypto.randomUUID(); - function start(modelPyName, args) { + function start(modelPyName, args, port) { logger.debug('logging model start'); - fetch(`${HOSTNAME}:${process.env.PORT}/${PREFIX}/log_model_start`, { + fetch(`${HOSTNAME}:${port}/${PREFIX}/log_model_start`, { method: 'post', body: JSON.stringify({ model_pyname: modelPyName, @@ -31,9 +31,9 @@ export default function investUsageLogger() { .catch((error) => logger.error(error)); } - function exit(status) { + function exit(status, port) { logger.debug('logging model exit'); - fetch(`${HOSTNAME}:${process.env.PORT}/${PREFIX}/log_model_exit`, { + fetch(`${HOSTNAME}:${port}/${PREFIX}/log_model_exit`, { method: 'post', body: JSON.stringify({ session_id: sessionId, diff --git a/workbench/src/main/setupInvestHandlers.js b/workbench/src/main/setupInvestHandlers.js index a2bb4cd71..762446a5b 100644 --- a/workbench/src/main/setupInvestHandlers.js +++ b/workbench/src/main/setupInvestHandlers.js @@ -88,6 +88,7 @@ export function setupInvestRunHandlers() { await writeInvestParameters(payload); let cmd; let cmdArgs; + let port; const plugins = settingsStore.get('plugins'); if (plugins && Object.keys(plugins).includes(modelRunName)) { cmd = settingsStore.get('mamba'); @@ -103,6 +104,7 @@ export function setupInvestRunHandlers() { modelRunName, `-d "${datastackPath}"`, ]; + port = settingsStore.get(`plugins.${modelRunName}.port`); } else { cmd = settingsStore.get('investExe'); cmdArgs = [ @@ -112,6 +114,7 @@ export function setupInvestRunHandlers() { 'run', modelRunName, `-d "${datastackPath}"`]; + port = settingsStore.get('core.port'); } logger.debug(`about to run model with command: ${cmd} ${cmdArgs}`); @@ -141,7 +144,7 @@ export function setupInvestRunHandlers() { ); event.reply(`invest-logging-${tabID}`, path.resolve(investLogfile)); if (!ELECTRON_DEV_MODE && !process.env.PUPPETEER) { - usageLogger.start(pyModuleName, args); + usageLogger.start(pyModuleName, args, port); } } } @@ -176,7 +179,7 @@ export function setupInvestRunHandlers() { }); }); if (!ELECTRON_DEV_MODE && !process.env.PUPPETEER) { - usageLogger.exit(investStdErr); + usageLogger.exit(investStdErr, port); } }); }); diff --git a/workbench/tests/main/main.test.js b/workbench/tests/main/main.test.js index 3fc064913..c0895b1d1 100644 --- a/workbench/tests/main/main.test.js +++ b/workbench/tests/main/main.test.js @@ -225,7 +225,8 @@ describe('createWindow', () => { }); describe('investUsageLogger', () => { - const expectedURL = `http://127.0.0.1:${process.env.PORT}/api/log_model_start`; + const PORT = 3000; + const expectedURL = `http://127.0.0.1:${PORT}/api/log_model_start`; beforeEach(() => { // the expected response const response = { @@ -244,12 +245,12 @@ describe('investUsageLogger', () => { const investStdErr = ''; const usageLogger = investUsageLogger(); - usageLogger.start(modelPyname, args); + usageLogger.start(modelPyname, args, PORT); expect(fetch.mock.calls).toHaveLength(1); expect(fetch.mock.calls[0][0]).toBe(expectedURL); const startPayload = JSON.parse(fetch.mock.calls[0][1].body); - usageLogger.exit(investStdErr); + usageLogger.exit(investStdErr, PORT); expect(fetch.mock.calls).toHaveLength(2); const exitPayload = JSON.parse(fetch.mock.calls[1][1].body); From eeb6bfb382a192f0e04ae819bc3d871b1d6aa6b8 Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Wed, 20 Nov 2024 10:38:56 -0500 Subject: [PATCH 3/6] No longer need PORT in test calls --- workbench/package.json | 4 ++-- workbench/tests/main/main.test.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/workbench/package.json b/workbench/package.json index 51a560000..77b66ea90 100644 --- a/workbench/package.json +++ b/workbench/package.json @@ -8,10 +8,10 @@ "start": "yarn build-main && yarn build:preload && concurrently --kill-others \"yarn serve\" \"electron .\"", "serve": "cross-env MODE=development node scripts/watch.js", "lint": "eslint --cache --color --ext .jsx,.js src", - "test": "cross-env PORT=56788 jest --runInBand --testPathIgnorePatterns /tests/binary_tests/ /tests/sampledata_linkcheck/", + "test": "cross-env jest --runInBand --testPathIgnorePatterns /tests/binary_tests/ /tests/sampledata_linkcheck/", "test-main": "jest --runInBand --testMatch **/tests/main/*.test.js", "test-renderer": "jest --runInBand --testMatch **/tests/renderer/*.test.js", - "test-flask": "cross-env PORT=56788 jest --runInBand --testMatch **/tests/invest/*.test.js", + "test-flask": "cross-env jest --runInBand --testMatch **/tests/invest/*.test.js", "test-electron-app": "jest --runInBand --testMatch **/tests/binary_tests/*.test.js", "test-sampledata-registry": "jest --runInBand --testMatch **/tests/sampledata_linkcheck/*.test.js", "postinstall": "electron-builder install-app-deps", diff --git a/workbench/tests/main/main.test.js b/workbench/tests/main/main.test.js index c0895b1d1..ffcae7436 100644 --- a/workbench/tests/main/main.test.js +++ b/workbench/tests/main/main.test.js @@ -225,6 +225,7 @@ describe('createWindow', () => { }); describe('investUsageLogger', () => { + // PORT is not used, but set the URL port const PORT = 3000; const expectedURL = `http://127.0.0.1:${PORT}/api/log_model_start`; beforeEach(() => { From 8331349f42fa6799ac42cb81ff165e42285542cb Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Wed, 20 Nov 2024 10:51:37 -0500 Subject: [PATCH 4/6] Fix comment --- workbench/tests/main/main.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workbench/tests/main/main.test.js b/workbench/tests/main/main.test.js index ffcae7436..03f61f779 100644 --- a/workbench/tests/main/main.test.js +++ b/workbench/tests/main/main.test.js @@ -225,7 +225,7 @@ describe('createWindow', () => { }); describe('investUsageLogger', () => { - // PORT is not used, but set the URL port + // Set default PORT for URL, but it's not used by the test. const PORT = 3000; const expectedURL = `http://127.0.0.1:${PORT}/api/log_model_start`; beforeEach(() => { From 5cc7ffe112f80cb16d3bfead51a5f80813332fdf Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Thu, 21 Nov 2024 09:33:00 -0500 Subject: [PATCH 5/6] remove cross-env, no longer needed --- workbench/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workbench/package.json b/workbench/package.json index 77b66ea90..78f7c08dd 100644 --- a/workbench/package.json +++ b/workbench/package.json @@ -8,10 +8,10 @@ "start": "yarn build-main && yarn build:preload && concurrently --kill-others \"yarn serve\" \"electron .\"", "serve": "cross-env MODE=development node scripts/watch.js", "lint": "eslint --cache --color --ext .jsx,.js src", - "test": "cross-env jest --runInBand --testPathIgnorePatterns /tests/binary_tests/ /tests/sampledata_linkcheck/", + "test": "jest --runInBand --testPathIgnorePatterns /tests/binary_tests/ /tests/sampledata_linkcheck/", "test-main": "jest --runInBand --testMatch **/tests/main/*.test.js", "test-renderer": "jest --runInBand --testMatch **/tests/renderer/*.test.js", - "test-flask": "cross-env jest --runInBand --testMatch **/tests/invest/*.test.js", + "test-flask": "jest --runInBand --testMatch **/tests/invest/*.test.js", "test-electron-app": "jest --runInBand --testMatch **/tests/binary_tests/*.test.js", "test-sampledata-registry": "jest --runInBand --testMatch **/tests/sampledata_linkcheck/*.test.js", "postinstall": "electron-builder install-app-deps", From 276632db06803c29b00d469f65ac2df0b95f99e7 Mon Sep 17 00:00:00 2001 From: dcdenu4 Date: Thu, 21 Nov 2024 09:53:08 -0500 Subject: [PATCH 6/6] add comment for shell set to true. --- workbench/src/main/setupAddRemovePlugin.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workbench/src/main/setupAddRemovePlugin.js b/workbench/src/main/setupAddRemovePlugin.js index 2676881df..09463b3b3 100644 --- a/workbench/src/main/setupAddRemovePlugin.js +++ b/workbench/src/main/setupAddRemovePlugin.js @@ -15,7 +15,8 @@ const logger = getLogger(__filename.split('/').slice(-1)[0]); * Spawn a child process and log its stdout, stderr, and any error in spawning. * * child_process.spawn is called with the provided cmd, args, and options, - * and the windowsHide option set to true. + * and the windowsHide option set to true. The shell option is set to true + * because spawn by default sets shell to false. * * Required properties missing from the store are initialized with defaults. * Invalid properties are reset to defaults.