From ec34a4ecc6d4a9891ccaa382538f8972d3f2fa7d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 Oct 2016 13:53:40 -0400 Subject: [PATCH] Fix tests by making sure to close optimistic file watchers. If a test process does not explicitly call process.exit, pathwatcher watchers may keep it alive indefinitely (either that, or there's a bug with the persistent:false option to fs.watchFile). This accidental immortality can be prevented by explicitly closing all watchers when we no longer have any interest in file change notifications. --- tools/fs/safe-watcher.js | 40 ++++++++++++++++++------- tools/runners/run-app.js | 5 ++++ tools/tests/old/test-bundler-assets.js | 5 ++++ tools/tests/old/test-bundler-options.js | 6 +++- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/tools/fs/safe-watcher.js b/tools/fs/safe-watcher.js index 78ea0309e9..16d91705e3 100644 --- a/tools/fs/safe-watcher.js +++ b/tools/fs/safe-watcher.js @@ -130,7 +130,7 @@ function startNewWatcher(absPath) { rewatch(); - return { + const entry = { callbacks, rewatch, @@ -153,18 +153,38 @@ function startNewWatcher(absPath) { // can avoid tearing anything down. return; } - - watchers[absPath] = null; - - if (watcher) { - watcher.close(); - watcher = null; - } - - unwatchFile(absPath, watchFileWrapper); + entry.close(); }, WATCHER_CLEANUP_DELAY_MS); + }, + + close() { + if (watchers[absPath] !== entry) return; + watchers[absPath] = null; + + if (watcherCleanupTimer) { + clearTimeout(watcherCleanupTimer); + watcherCleanupTimer = null; + } + + if (watcher) { + watcher.close(); + watcher = null; + } + + unwatchFile(absPath, watchFileWrapper); } }; + + return entry; +} + +export function closeAllWatchers() { + Object.keys(watchers).forEach(absPath => { + const entry = watchers[absPath]; + if (entry) { + entry.close(); + } + }); } function watchLibraryWatch(absPath, callback) { diff --git a/tools/runners/run-app.js b/tools/runners/run-app.js index e0568e6cdc..f50ea91c74 100644 --- a/tools/runners/run-app.js +++ b/tools/runners/run-app.js @@ -13,6 +13,7 @@ var Profile = require('../tool-env/profile.js').Profile; var release = require('../packaging/release.js'); import * as cordova from '../cordova'; import { CordovaBuilder } from '../cordova/builder.js'; +import { closeAllWatchers } from "../fs/safe-watcher.js"; // Parse out s as if it were a bash command line. var bashParse = function (s) { @@ -980,6 +981,10 @@ _.extend(AppRunner.prototype, { break; } + // Allow the process to exit normally, since optimistic file watchers + // may be keeping the event loop busy. + closeAllWatchers(); + // Giving up for good. self._cleanUpPromises(); diff --git a/tools/tests/old/test-bundler-assets.js b/tools/tests/old/test-bundler-assets.js index 3a2ebdccc1..953ab0b9e1 100644 --- a/tools/tests/old/test-bundler-assets.js +++ b/tools/tests/old/test-bundler-assets.js @@ -10,6 +10,7 @@ var release = require('../../packaging/release.js'); var catalog = require('../../packaging/catalog/catalog.js'); var buildmessage = require('../../utils/buildmessage.js'); var projectContextModule = require('../../project-context.js'); +var safeWatcher = require("../../fs/safe-watcher.js"); var lastTmpDir = null; var tmpDir = function () { @@ -156,4 +157,8 @@ Fiber(function () { console.log('\nBundle can be found at ' + lastTmpDir); process.exit(1); } + + // Allow the process to exit normally, since optimistic file watchers + // may be keeping the event loop busy. + safeWatcher.closeAllWatchers(); }).run(); diff --git a/tools/tests/old/test-bundler-options.js b/tools/tests/old/test-bundler-options.js index 23daa42588..12b9a8c5e8 100644 --- a/tools/tests/old/test-bundler-options.js +++ b/tools/tests/old/test-bundler-options.js @@ -9,7 +9,7 @@ var catalog = require('../../packaging/catalog/catalog.js'); var buildmessage = require('../../utils/buildmessage.js'); var isopackets = require('../../tool-env/isopackets.js'); var projectContextModule = require('../../project-context.js'); - +var safeWatcher = require("../../fs/safe-watcher.js"); var lastTmpDir = null; var tmpDir = function () { @@ -165,4 +165,8 @@ Fiber(function () { console.log('\nBundle can be found at ' + lastTmpDir); process.exit(1); } + + // Allow the process to exit normally, since optimistic file watchers + // may be keeping the event loop busy. + safeWatcher.closeAllWatchers(); }).run();