Merge pull request #1364 from davemfish/bugfix/WB-1362-macos-pptr

Workbench: cleanup the process shutdown routines

workbench/src/main/createPythonFlaskProcess.js

@@ -1,4 +1,4 @@
-import { spawn, exec } from 'child_process';
+import { spawn, execSync } from 'child_process';
 
 import fetch from 'node-fetch';
 
@@ -88,26 +88,12 @@ export async function shutdownPythonProcess(subprocess) {
       subprocess.kill();
     } else {
       const { pid } = subprocess;
-      exec(`taskkill /pid ${pid} /t /f`);
+      execSync(`taskkill /pid ${pid} /t /f`);
     }
   } catch (error) {
     // if the process was already killed by some other means
     logger.debug(error);
+  } finally {
+    Promise.resolve();
   }
-
-  // If we return too quickly, it seems the electron app is allowed
-  // to quit before the subprocess is killed, and the subprocess remains
-  // open. Here we poll a flask endpoint and resolve only when it
-  // gives ECONNREFUSED.
-  return fetch(`${HOSTNAME}:${process.env.PORT}/ready`, {
-    method: 'get',
-  })
-    .then(async () => {
-      await new Promise((resolve) => setTimeout(resolve, 300));
-      return shutdownPythonProcess(subprocess);
-    })
-    .catch(() => {
-      logger.debug('flask server is closed');
-      return Promise.resolve();
-    });
 }

workbench/tests/binary_tests/puppet.test.js

@@ -16,6 +16,7 @@ import { APP_HAS_RUN_TOKEN } from '../../src/main/setupCheckFirstRun';
 
 jest.setTimeout(240000);
 const PORT = 9009;
+const WAIT_TO_CLICK = 300; // ms
 let ELECTRON_PROCESS;
 let BROWSER;
 
@@ -133,13 +134,15 @@ beforeEach(() => {
 
 afterEach(async () => {
   try {
+    const pages = await BROWSER.pages();
+    await Promise.all(pages.map(page => page.close()));
     await BROWSER.close();
   } catch (error) {
     console.log(BINARY_PATH);
     console.error(error);
+    // Normally BROWSER.close() will kill this process
+    ELECTRON_PROCESS.kill();
   }
-  ELECTRON_PROCESS.removeAllListeners();
-  ELECTRON_PROCESS.kill();
 });
 
 test('Run a real invest model', async () => {
@@ -164,6 +167,7 @@ test('Run a real invest model', async () => {
   const downloadModal = await page.waitForSelector('.modal-dialog');
   const downloadModalCancel = await downloadModal.waitForSelector(
     'aria/[name="Cancel"][role="button"]');
+  await page.waitForTimeout(WAIT_TO_CLICK); // waiting for click handler to be ready
   await downloadModalCancel.click();
   // We need to get the modelButton from w/in this list-group because there
   // are buttons with the same name in the Recent Jobs container.
@@ -232,15 +236,17 @@ test('Check local userguide links', async () => {
   const downloadModal = await page.waitForSelector('.modal-dialog');
   const downloadModalCancel = await downloadModal.waitForSelector(
     'aria/[name="Cancel"][role="button"]');
+  await page.waitForTimeout(WAIT_TO_CLICK); // waiting for click handler to be ready
   await downloadModalCancel.click();
 
   const investList = await page.waitForSelector('.invest-list-group');
   const modelButtons = await investList.$$('aria/[role="button"]');
 
+  await page.waitForTimeout(WAIT_TO_CLICK); // first btn click does not register w/o this pause
   for (const btn of modelButtons) {
     await btn.click();
     const link = await page.waitForSelector('text/User\'s Guide');
-    await page.waitForTimeout(300); // link.click() not working w/o this pause
+    await page.waitForTimeout(WAIT_TO_CLICK); // link.click() not working w/o this pause
     const hrefHandle = await link.getProperty('href');
     const hrefValue = await hrefHandle.jsonValue();
     await link.click();