test: don't use process.exit to end tests

Newer versions of `tap` run more asynchronously, so sometimes using `process.nextTick(process.exit)`
to end a test would prevent the test from completing correctly. Removing all instances of
`process.nextTick(process.exit)` put tests into three categories:
* the test still worked correctly -- no fixup needed.
* the test would hang because the VM's `_steppingInterval` was keeping
  Node alive. These tests call a new `quit()` method which ends the
  stepping interval.
* the `load-extensions` test needed special attention because the "Video
  Sensing" extension starts its own loop using `setTimeout`. I added a
  `_stopLoop()` method on the extension and directly call that from the
  test. I'm not completely happy with this solution but anything more
  general would likely require a change to the extension spec, so I'm
  leaving that as a followup task.

Author: cwillisf

src/engine/runtime.js

@@ -608,7 +608,7 @@ class Runtime extends EventEmitter {
     static get PERIPHERAL_LIST_UPDATE () {
         return 'PERIPHERAL_LIST_UPDATE';
     }
-    
+
     /**
      * Event name for when the user picks a bluetooth device to connect to
      * via Companion Device Manager (CDM)
@@ -2578,6 +2578,15 @@ class Runtime extends EventEmitter {
         this.emit(Runtime.RUNTIME_STARTED);
     }
 
+    /**
+     * Quit the Runtime, clearing any handles which might keep the process alive.
+     * Do not use the runtime after calling this method. This method is meant for test shutdown.
+     */
+    quit () {
+        clearInterval(this._steppingInterval);
+        this._steppingInterval = null;
+    }
+
     /**
      * Turn on profiling.
      * @param {Profiler/FrameCallback} onFrame A callback handle passed a

src/extension-support/extension-manager.js

@@ -81,8 +81,8 @@ class ExtensionManager {
         this.pendingWorkers = [];
 
         /**
-         * Set of loaded extension URLs/IDs (equivalent for built-in extensions).
-         * @type {Set.<string>}
+         * Map of loaded extension URLs/IDs (equivalent for built-in extensions) to service name.
+         * @type {Map.<string,string>}
          * @private
          */
         this._loadedExtensions = new Map();

src/extensions/scratch3_video_sensing/index.js

@@ -231,7 +231,8 @@ class Scratch3VideoSensingBlocks {
      * @private
      */
     _loop () {
-        setTimeout(this._loop.bind(this), Math.max(this.runtime.currentStepTime, Scratch3VideoSensingBlocks.INTERVAL));
+        const loopTime = Math.max(this.runtime.currentStepTime, Scratch3VideoSensingBlocks.INTERVAL);
+        this._loopInterval = setTimeout(this._loop.bind(this), loopTime);
 
         // Add frame to detector
         const time = Date.now();
@@ -251,6 +252,13 @@ class Scratch3VideoSensingBlocks {
         }
     }
 
+    /**
+     * Stop the video sampling loop. Only used for testing.
+     */
+    _stopLoop () {
+        clearTimeout(this._loopInterval);
+    }
+
     /**
      * Create data for a menu in scratch-blocks format, consisting of an array
      * of objects with text and value properties. The text is a translated

src/virtual-machine.js

@@ -175,6 +175,14 @@ class VirtualMachine extends EventEmitter {
         this.runtime.start();
     }
 
+    /**
+     * Quit the VM, clearing any handles which might keep the process alive.
+     * Do not use the runtime after calling this method. This method is meant for test shutdown.
+     */
+    quit () {
+        this.runtime.quit();
+    }
+
     /**
      * "Green flag" handler - start all threads starting with a green flag.
      */

test/integration/addSprite.js

@@ -27,8 +27,8 @@ test('default cat', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length === 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     vm.start();

test/integration/block_to_workspace_comment_import.js

@@ -33,8 +33,8 @@ test('importing sb2 project where block comment is converted to workspace commen
         const invalidComments = targetComments.filter(comment => typeof comment.blockId === 'number');
         t.equal(invalidComments.length, 0);
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/block_to_workspace_comment_import_no_scripts.js

@@ -38,8 +38,8 @@ test('importing sb2 project where block comment is converted to workspace commen
         const targetBlocks = Object.values(target.blocks._blocks);
         t.equal(targetBlocks.length, 0);
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/broadcast_special_chars_sb2.js

@@ -62,8 +62,8 @@ test('importing sb2 project with special chars in message names', t => {
         t.equal(catMessageBlocks[0].fields.BROADCAST_OPTION.id, ltPerfectMessageId);
         t.equal(catMessageBlocks[1].fields.BROADCAST_OPTION.id, abMessageId);
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/broadcast_special_chars_sb3.js

@@ -62,8 +62,8 @@ test('importing sb3 project with special chars in message names', t => {
         t.equal(catMessageBlocks[0].fields.BROADCAST_OPTION.id, ltPerfectMessageId);
         t.equal(catMessageBlocks[1].fields.BROADCAST_OPTION.id, abMessageId);
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/clone-cleanup.js

@@ -65,8 +65,8 @@ test('clone-cleanup', t => {
             // The second batch of clones has deleted themselves; everything is finished
             verifyCounts(0, 0);
 
+            vm.quit();
             t.end();
-            process.nextTick(process.exit);
             break;
         }
     };

test/integration/cloud_variables_sb2.js

@@ -37,6 +37,7 @@ test('importing an sb2 project with cloud variables', t => {
         // when the message is being sent to the server rather than on the client
         t.equal(variable.isCloud, true);
 
+        vm.quit();
         t.end();
     });
 });
@@ -60,6 +61,7 @@ test('importing an sb2 project with cloud variables at the limit for a project',
         // All of the 8 stage variables should be cloud variables
         t.equal(stageVars.filter(v => v.isCloud).length, 10);
 
+        vm.quit();
         t.end();
     });
 });
@@ -85,6 +87,7 @@ test('importing an sb2 project with cloud variables exceeding the limit for a pr
         // Only 8 of the variables should have the isCloud flag set to true
         t.equal(stageVars.filter(v => v.isCloud).length, 10);
 
+        vm.quit();
         t.end();
     });
 });
@@ -115,6 +118,7 @@ test('importing one project after the other resets cloud variable limit', t => {
 
             t.equal(vm.runtime.canAddCloudVariable(), true);
 
+            vm.quit();
             t.end();
         });
     });
@@ -145,8 +149,7 @@ test('local cloud variables get imported as regular variables', t => {
         t.equal(spriteVars.length, 1);
         t.equal(spriteVars[0].isCloud, false);
 
+        vm.quit();
         t.end();
-
-        process.nextTick(process.exit); // This is needed because this is the end of the last test in this file!!!
     });
 });

test/integration/cloud_variables_sb3.js

@@ -35,6 +35,7 @@ test('importing an sb3 project with cloud variables', t => {
         t.equal(Number(variable.value), 100);
         t.equal(variable.isCloud, true);
 
+        vm.quit();
         t.end();
     });
 });
@@ -58,6 +59,7 @@ test('importing an sb3 project with cloud variables at the limit for a project',
         // All of the 10 stage variables should be cloud variables
         t.equal(stageVars.filter(v => v.isCloud).length, 10);
 
+        vm.quit();
         t.end();
     });
 });
@@ -83,6 +85,7 @@ test('importing an sb3 project with cloud variables exceeding the limit for a pr
         // Only 8 of the variables should have the isCloud flag set to true
         t.equal(stageVars.filter(v => v.isCloud).length, 10);
 
+        vm.quit();
         t.end();
     });
 });
@@ -111,6 +114,7 @@ test('importing one project after the other resets cloud variable limit', t => {
 
             t.equal(vm.runtime.canAddCloudVariable(), true);
 
+            vm.quit();
             t.end();
         });
     });
@@ -141,8 +145,7 @@ test('local cloud variables get imported as regular variables', t => {
         t.equal(spriteVars.length, 1);
         t.equal(spriteVars[0].isCloud, false);
 
+        vm.quit();
         t.end();
-
-        process.nextTick(process.exit); // This is needed because this is the end of the last test in this file!!!
     });
 });

test/integration/comments.js

@@ -70,8 +70,8 @@ test('importing sb2 project with comments', t => {
         t.equal(stopAllBlock.comment, blockComments[4].id);
         t.equal(stopAllBlock.opcode, 'control_stop');
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/comments_sb3.js

@@ -70,8 +70,8 @@ test('load an sb3 project with comments', t => {
         t.equal(stopAllBlock.comment, blockComments[4].id);
         t.equal(stopAllBlock.opcode, 'control_stop');
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/complex.js

@@ -19,8 +19,8 @@ test('complex', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length === 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Manipulate each target

test/integration/control.js

@@ -15,8 +15,8 @@ test('control', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length > 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/data.js

@@ -14,8 +14,8 @@ test('data', t => {
     // Evaluate playground data and exit
     vm.on('playgroundData', () => {
         // @todo Additional tests
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/delete-and-restore-sprite.js

@@ -27,8 +27,8 @@ test('default cat', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length === 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     vm.start();

test/integration/event.js

@@ -15,8 +15,8 @@ test('event', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length > 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/execute.js

@@ -66,6 +66,8 @@ fs.readdirSync(executeDir)
             log.suggest.deny('vm', 'error');
             t.tearDown(() => log.suggest.clear());
 
+            const vm = new VirtualMachine();
+
             // Map string messages to tap reporting methods. This will be used
             // with events from scratch's runtime emitted on block instructions.
             let didPlan;
@@ -86,6 +88,7 @@ fs.readdirSync(executeDir)
                 },
                 end () {
                     didEnd = true;
+                    vm.quit();
                     t.end();
                 }
             };
@@ -100,7 +103,6 @@ fs.readdirSync(executeDir)
                 return reporters.comment(text);
             };
 
-            const vm = new VirtualMachine();
             vm.attachStorage(makeTestStorage());
 
             // Start the VM and initialize some vm properties.
@@ -138,6 +140,7 @@ fs.readdirSync(executeDir)
                     // it can be resolved.
                     if (!didEnd) {
                         t.fail('did not say "end"');
+                        vm.quit();
                         t.end();
                     }
                 });

test/integration/hat-execution-order.js

@@ -20,8 +20,8 @@ test('complex', t => {
         const results = vm.runtime.targets[0].variables[resultKey].value;
         t.deepEqual(results, ['3', '2', '1', 'stage']);
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/import-sb.js

@@ -15,8 +15,8 @@ test('default', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length === 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/import-sb2-from-object.js

@@ -15,8 +15,8 @@ test('default', t => {
     vm.on('playgroundData', e => {
         const threads = JSON.parse(e.threads);
         t.ok(threads.length === 0);
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run

test/integration/list-monitor-rename.js

@@ -31,8 +31,8 @@ test('importing sb3 project with incorrect list monitor name', t => {
             t.equal(monitorBlock.fields.LIST.value, renamedListName);
         }
 
+        vm.quit();
         t.end();
-        process.nextTick(process.exit);
     });
 
     // Start VM, load project, and run