From a18e1d654fe8f3b4df3cf62f4a572d06acf4c64c Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Thu, 16 Aug 2018 18:56:34 +0300 Subject: [PATCH 1/5] Reduce number of created temp dirs for profiles (closes #2735) --- .../built-in/chrome/create-temp-profile.js | 10 +- src/browser/provider/built-in/chrome/index.js | 3 + .../provider/built-in/chrome/local-chrome.js | 2 +- .../provider/built-in/chrome/runtime-info.js | 18 +-- .../built-in/firefox/create-temp-profile.js | 8 +- .../provider/built-in/firefox/index.js | 3 + .../built-in/firefox/local-firefox.js | 2 +- src/utils/promisified-functions.js | 1 + src/utils/temp-directory.js | 131 ++++++++++++++++++ 9 files changed, 148 insertions(+), 30 deletions(-) create mode 100644 src/utils/temp-directory.js diff --git a/src/browser/provider/built-in/chrome/create-temp-profile.js b/src/browser/provider/built-in/chrome/create-temp-profile.js index 63efba2d6b3..073a82a10ce 100644 --- a/src/browser/provider/built-in/chrome/create-temp-profile.js +++ b/src/browser/provider/built-in/chrome/create-temp-profile.js @@ -1,13 +1,11 @@ import path from 'path'; -import tmp from 'tmp'; +import TempDirectory from '../../../../utils/temp-directory'; import { writeFile, ensureDir } from '../../../../utils/promisified-functions'; export default async function (proxyHostName) { - tmp.setGracefulCleanup(); - - const tempDir = tmp.dirSync({ unsafeCleanup: true }); - const profileDirName = path.join(tempDir.name, 'Default'); + const tempDir = await TempDirectory.createDirectory('chrome-profile'); + const profileDirName = path.join(tempDir.path, 'Default'); await ensureDir(profileDirName); @@ -39,7 +37,7 @@ export default async function (proxyHostName) { }; await writeFile(path.join(profileDirName, 'Preferences'), JSON.stringify(preferences)); - await writeFile(path.join(tempDir.name, 'First Run'), ''); + await writeFile(path.join(tempDir.path, 'First Run'), ''); return tempDir; } diff --git a/src/browser/provider/built-in/chrome/index.js b/src/browser/provider/built-in/chrome/index.js index 12a4ca4c620..8751c3671b0 100644 --- a/src/browser/provider/built-in/chrome/index.js +++ b/src/browser/provider/built-in/chrome/index.js @@ -46,6 +46,9 @@ export default { if (OS.mac || runtimeInfo.config.headless) await stopLocalChrome(runtimeInfo); + if (runtimeInfo.tempProfileDir) + runtimeInfo.tempProfileDir.dispose(); + delete this.openedBrowsers[browserId]; }, diff --git a/src/browser/provider/built-in/chrome/local-chrome.js b/src/browser/provider/built-in/chrome/local-chrome.js index 62649cbf8ad..ca199035542 100644 --- a/src/browser/provider/built-in/chrome/local-chrome.js +++ b/src/browser/provider/built-in/chrome/local-chrome.js @@ -9,7 +9,7 @@ function buildChromeArgs (config, cdpPort, platformArgs, profileDir) { return [] .concat( cdpPort ? [`--remote-debugging-port=${cdpPort}`] : [], - !config.userProfile ? [`--user-data-dir=${profileDir.name}`] : [], + !config.userProfile ? [`--user-data-dir=${profileDir.path}`] : [], config.headless ? ['--headless'] : [], config.userArgs ? [config.userArgs] : [], platformArgs ? [platformArgs] : [] diff --git a/src/browser/provider/built-in/chrome/runtime-info.js b/src/browser/provider/built-in/chrome/runtime-info.js index ab291b099f1..2f47c3de847 100644 --- a/src/browser/provider/built-in/chrome/runtime-info.js +++ b/src/browser/provider/built-in/chrome/runtime-info.js @@ -3,25 +3,9 @@ import getConfig from './config'; import createTempProfile from './create-temp-profile'; -var commonTempProfile = null; - -async function getTempProfile (proxyHostName, config) { - - var tempProfile = commonTempProfile; - var shouldUseCommonProfile = !config.headless && !config.emulation; - - if (!shouldUseCommonProfile || !commonTempProfile) - tempProfile = await createTempProfile(proxyHostName); - - if (shouldUseCommonProfile && !commonTempProfile) - commonTempProfile = tempProfile; - - return tempProfile; -} - export default async function (proxyHostName, configString) { var config = getConfig(configString); - var tempProfileDir = !config.userProfile ? await getTempProfile(proxyHostName, config) : null; + var tempProfileDir = !config.userProfile ? await createTempProfile(proxyHostName, config) : null; var cdpPort = config.cdpPort || (!config.userProfile ? await getFreePort() : null); return { config, cdpPort, tempProfileDir }; diff --git a/src/browser/provider/built-in/firefox/create-temp-profile.js b/src/browser/provider/built-in/firefox/create-temp-profile.js index c30e7f885a2..5d5283b961b 100644 --- a/src/browser/provider/built-in/firefox/create-temp-profile.js +++ b/src/browser/provider/built-in/firefox/create-temp-profile.js @@ -1,5 +1,5 @@ import path from 'path'; -import tmp from 'tmp'; +import TempDirectory from '../../../../utils/temp-directory'; import { writeFile } from '../../../../utils/promisified-functions'; @@ -54,11 +54,9 @@ async function generatePreferences (profileDir, { marionettePort, config }) { } export default async function (runtimeInfo) { - tmp.setGracefulCleanup(); + const tmpDir = await TempDirectory.createDirectory('firefox-profile'); - const tmpDir = tmp.dirSync({ unsafeCleanup: true }); - - await generatePreferences(tmpDir.name, runtimeInfo); + await generatePreferences(tmpDir.path, runtimeInfo); return tmpDir; } diff --git a/src/browser/provider/built-in/firefox/index.js b/src/browser/provider/built-in/firefox/index.js index bbb3aa1c654..c3738e5766d 100644 --- a/src/browser/provider/built-in/firefox/index.js +++ b/src/browser/provider/built-in/firefox/index.js @@ -53,6 +53,9 @@ export default { if (OS.mac && !config.headless) await stopLocalFirefox(runtimeInfo); + if (runtimeInfo.tempProfileDir) + runtimeInfo.tempProfileDir.dispose(); + delete this.openedBrowsers[browserId]; }, diff --git a/src/browser/provider/built-in/firefox/local-firefox.js b/src/browser/provider/built-in/firefox/local-firefox.js index 75aff2cb823..efaafa6f8a0 100644 --- a/src/browser/provider/built-in/firefox/local-firefox.js +++ b/src/browser/provider/built-in/firefox/local-firefox.js @@ -18,7 +18,7 @@ function buildFirefoxArgs (config, platformArgs, { marionettePort, tempProfileDi return [] .concat( marionettePort ? ['-marionette'] : [], - !config.userProfile ? ['-no-remote', '-new-instance', `-profile "${tempProfileDir.name}"`] : [], + !config.userProfile ? ['-no-remote', '-new-instance', `-profile "${tempProfileDir.path}"`] : [], config.headless ? ['-headless'] : [], config.userArgs ? [config.userArgs] : [], platformArgs ? [platformArgs] : [] diff --git a/src/utils/promisified-functions.js b/src/utils/promisified-functions.js index a6aa010a921..0903d2f9039 100644 --- a/src/utils/promisified-functions.js +++ b/src/utils/promisified-functions.js @@ -6,6 +6,7 @@ import promisify from './promisify'; export const ensureDir = promisify(mkdirp); +export const readDir = promisify(fs.readdir); export const stat = promisify(fs.stat); export const writeFile = promisify(fs.writeFile); export const readFile = promisify(fs.readFile); diff --git a/src/utils/temp-directory.js b/src/utils/temp-directory.js new file mode 100644 index 00000000000..768a5136363 --- /dev/null +++ b/src/utils/temp-directory.js @@ -0,0 +1,131 @@ +import debug from 'debug'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import setupExitHook from 'async-exit-hook'; +import tmp from 'tmp'; +import { ensureDir, readDir } from '../utils/promisified-functions'; + + +const TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe-tmp'); +const LOCKFILE_NAME = '.testcafe-lockfile'; + +const USED_TEMP_DIRS = {}; + +const DEBUG_LOGGER = debug('testcafe:utils:temp-directory'); + +class LockFile { + constructor (dirPath) { + this.path = path.join(dirPath, LOCKFILE_NAME); + } + + init () { + try { + const fd = fs.openSync(this.path, 'wx'); + + fs.closeSync(fd); + + return true; + } + catch (e) { + DEBUG_LOGGER('Failed to init lockfile ' + this.path); + DEBUG_LOGGER(e); + + return false; + } + } + + dispose () { + try { + fs.unlinkSync(this.path); + } + catch (e) { + DEBUG_LOGGER('Failed to dispose lockfile ' + this.path); + DEBUG_LOGGER(e); + } + } +} + +export default class TempDirectory { + constructor (namePrefix) { + this.namePrefix = namePrefix; + + this.path = ''; + this.lockFile = null; + } + + async _getTmpDirsList () { + const tmpDirNames = await readDir(TESTCAFE_TMP_DIRS_ROOT); + + return tmpDirNames + .filter(tmpDir => !USED_TEMP_DIRS[tmpDir]) + .filter(tmpDir => path.basename(tmpDir).startsWith(this.namePrefix)); + } + + async _findFreeTmpDir (tmpDirNames) { + for (const tmpDirName of tmpDirNames) { + const tmpDirPath = path.join(TESTCAFE_TMP_DIRS_ROOT, tmpDirName); + + const lockFile = new LockFile(tmpDirPath); + + if (lockFile.init()) { + this.path = tmpDirPath; + this.lockFile = lockFile; + + return true; + } + } + + return false; + } + + async _createNewTmpDir () { + this.path = tmp.tmpNameSync({ dir: TESTCAFE_TMP_DIRS_ROOT, prefix: this.namePrefix + '-' }); + + await ensureDir(this.path); + + this.lockFile = new LockFile(this.path); + + this.lockFile.init(); + } + + static async createDirectory (prefix) { + const tmpDir = new TempDirectory(prefix); + + await tmpDir.init(); + + return tmpDir; + } + + static disposeDirectories () { + Object.values(USED_TEMP_DIRS).forEach(tmpDir => tmpDir.dispose()); + } + + async init () { + await ensureDir(TESTCAFE_TMP_DIRS_ROOT); + + const tmpDirNames = await this._getTmpDirsList(this.namePrefix); + + DEBUG_LOGGER('Found temp directories:', tmpDirNames); + + const existingTmpDirFound = await this._findFreeTmpDir(tmpDirNames); + + if (!existingTmpDirFound) + await this._createNewTmpDir(); + + DEBUG_LOGGER('Temp directory path: ', this.path); + + USED_TEMP_DIRS[this.path] = this; + } + + dispose () { + if (!USED_TEMP_DIRS[this.path]) + return; + + this.lockFile.dispose(); + + delete USED_TEMP_DIRS[this.path]; + } +} + +setupExitHook(TempDirectory.disposeDirectories); From 95a21d580bb5fd0d6e4a6baa5d7613aab998f812 Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Fri, 17 Aug 2018 11:30:48 +0300 Subject: [PATCH 2/5] Add test --- src/utils/temp-directory.js | 42 ++++++++++++++++++++++++++--- test/functional/assertion-helper.js | 4 ++- test/server/util-test.js | 39 +++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/utils/temp-directory.js b/src/utils/temp-directory.js index 768a5136363..8108b2858ce 100644 --- a/src/utils/temp-directory.js +++ b/src/utils/temp-directory.js @@ -7,8 +7,13 @@ import tmp from 'tmp'; import { ensureDir, readDir } from '../utils/promisified-functions'; -const TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe-tmp'); +// NOTE: mutable for testing purposes +let TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe-tmp'); + const LOCKFILE_NAME = '.testcafe-lockfile'; +const DEFAULT_NAME_PREFIX = 'tmp'; + +const STALE_LOCKFILE_AGE = 2 * 24 * 60 * 60 * 1000; const USED_TEMP_DIRS = {}; @@ -19,7 +24,7 @@ class LockFile { this.path = path.join(dirPath, LOCKFILE_NAME); } - init () { + _openLockFile () { try { const fd = fs.openSync(this.path, 'wx'); @@ -35,6 +40,26 @@ class LockFile { } } + _isStaleLockFile () { + const currentMs = Date.now(); + + try { + const { mtimeMs } = fs.statSync(this.path); + + return currentMs - mtimeMs > STALE_LOCKFILE_AGE; + } + catch (e) { + DEBUG_LOGGER('Failed to check status of lockfile ' + this.path); + DEBUG_LOGGER(e); + + return false; + } + } + + init () { + return this._openLockFile() || this._isStaleLockFile(); + } + dispose () { try { fs.unlinkSync(this.path); @@ -48,7 +73,7 @@ class LockFile { export default class TempDirectory { constructor (namePrefix) { - this.namePrefix = namePrefix; + this.namePrefix = namePrefix || DEFAULT_NAME_PREFIX; this.path = ''; this.lockFile = null; @@ -126,6 +151,17 @@ export default class TempDirectory { delete USED_TEMP_DIRS[this.path]; } + + // NOTE: for testing purposes + static get TEMP_DIRECTORIES_ROOT () { + return TESTCAFE_TMP_DIRS_ROOT; + } + + static set TEMP_DIRECTORIES_ROOT (value) { + TESTCAFE_TMP_DIRS_ROOT = value; + + return value; + } } setupExitHook(TempDirectory.disposeDirectories); diff --git a/test/functional/assertion-helper.js b/test/functional/assertion-helper.js index 439c31b6e52..5b6291c1e70 100644 --- a/test/functional/assertion-helper.js +++ b/test/functional/assertion-helper.js @@ -311,7 +311,9 @@ exports.isScreenshotsEqual = function (customPath, referenceImagePathGetter) { exports.removeScreenshotDir = function () { if (isDirExists(SCREENSHOTS_PATH)) - del(SCREENSHOTS_PATH); + return del(SCREENSHOTS_PATH); + + return Promise.resolve(); }; exports.SCREENSHOTS_PATH = SCREENSHOTS_PATH; diff --git a/test/server/util-test.js b/test/server/util-test.js index 66138f4f852..371a3136ba6 100644 --- a/test/server/util-test.js +++ b/test/server/util-test.js @@ -1,8 +1,14 @@ const path = require('path'); +const fs = require('fs'); +const del = require('del'); const expect = require('chai').expect; const correctFilePath = require('../../lib/utils/correct-file-path'); const escapeUserAgent = require('../../lib/utils/escape-user-agent'); const parseFileList = require('../../lib/utils/parse-file-list'); +const TempDirectory = require('../../lib/utils/temp-directory'); + + +const TMP_ROOT = path.join(process.cwd(), '__tmp__'); describe('Utils', () => { it('Correct File Path', () => { @@ -62,4 +68,37 @@ describe('Utils', () => { }); }); }); + + describe('Temp Directory', function () { + const savedTmpRoot = TempDirectory.TEMP_DIRECTORIES_ROOT; + + beforeEach(() => { + TempDirectory.TEMP_DIRECTORIES_ROOT = TMP_ROOT; + + return del(TMP_ROOT); + }); + + afterEach(() => { + TempDirectory.TEMP_DIRECTORIES_ROOT = savedTmpRoot; + + return del(TMP_ROOT); + }); + + it('Should reuse existing temp directories', function () { + const tempDir1 = new TempDirectory(); + const tempDir2 = new TempDirectory(); + const tempDir3 = new TempDirectory(); + + return tempDir1 + .init() + .then(() => tempDir2.init()) + .then(() => tempDir1.dispose()) + .then(() => tempDir3.init()) + .then(() => { + const subDirs = fs.readdirSync(TempDirectory.TEMP_DIRECTORIES_ROOT); + + expect(subDirs.length).eql(2); + }); + }); + }); }); From 15231bee9b1323b69cbd203972e22fc150800033 Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Fri, 17 Aug 2018 17:16:06 +0300 Subject: [PATCH 3/5] Remove temp dirs in a separate process --- src/browser/provider/built-in/chrome/index.js | 2 +- .../provider/built-in/firefox/index.js | 2 +- .../provider/utils/kill-browser-process.js | 33 ++++- .../temp-directory/cleanup-process/index.js | 137 ++++++++++++++++++ .../temp-directory/cleanup-process/worker.js | 73 ++++++++++ .../index.js} | 38 ++++- test/server/util-test.js | 23 ++- 7 files changed, 292 insertions(+), 16 deletions(-) create mode 100644 src/utils/temp-directory/cleanup-process/index.js create mode 100644 src/utils/temp-directory/cleanup-process/worker.js rename src/utils/{temp-directory.js => temp-directory/index.js} (81%) diff --git a/src/browser/provider/built-in/chrome/index.js b/src/browser/provider/built-in/chrome/index.js index 8751c3671b0..cfd0cc3ed3b 100644 --- a/src/browser/provider/built-in/chrome/index.js +++ b/src/browser/provider/built-in/chrome/index.js @@ -47,7 +47,7 @@ export default { await stopLocalChrome(runtimeInfo); if (runtimeInfo.tempProfileDir) - runtimeInfo.tempProfileDir.dispose(); + await runtimeInfo.tempProfileDir.dispose(); delete this.openedBrowsers[browserId]; }, diff --git a/src/browser/provider/built-in/firefox/index.js b/src/browser/provider/built-in/firefox/index.js index c3738e5766d..92fed99c99d 100644 --- a/src/browser/provider/built-in/firefox/index.js +++ b/src/browser/provider/built-in/firefox/index.js @@ -54,7 +54,7 @@ export default { await stopLocalFirefox(runtimeInfo); if (runtimeInfo.tempProfileDir) - runtimeInfo.tempProfileDir.dispose(); + await runtimeInfo.tempProfileDir.dispose(); delete this.openedBrowsers[browserId]; }, diff --git a/src/browser/provider/utils/kill-browser-process.js b/src/browser/provider/utils/kill-browser-process.js index 563d02317e7..f875aa2de33 100644 --- a/src/browser/provider/utils/kill-browser-process.js +++ b/src/browser/provider/utils/kill-browser-process.js @@ -1,12 +1,37 @@ +import { spawn } from 'child_process'; import OS from 'os-family'; -import { findProcess, killProcess, exec } from '../../../utils/promisified-functions'; +import promisifyEvent from 'promisify-event'; +import Promise from 'pinkie'; +import { findProcess, killProcess } from '../../../utils/promisified-functions'; const BROWSER_CLOSING_TIMEOUT = 5; +async function runWMIC (args) { + const wmicProcess = spawn('wmic.exe', args, { detached: true }); + + let wmicOutput = ''; + + wmicProcess.stdout.on('data', data => { + wmicOutput += data.toString(); + }); + + try { + await Promise.race([ + promisifyEvent(wmicProcess.stdout, 'end'), + promisifyEvent(wmicProcess, 'error') + ]); + + return wmicOutput; + } + catch (e) { + return ''; + } +} + async function findProcessWin (processOptions) { - var cmd = `wmic process where "commandline like '%${processOptions.arguments}%' and name <> 'cmd.exe' and name <> 'wmic.exe'" get processid`; - var wmicOutput = await exec(cmd); + var wmicArgs = ['process', 'where', `commandline like '%${processOptions.arguments}%' and name <> 'cmd.exe' and name <> 'wmic.exe'`, 'get', 'processid']; + var wmicOutput = await runWMIC(wmicArgs); var processList = wmicOutput.split(/\s*\n/); processList = processList @@ -18,7 +43,7 @@ async function findProcessWin (processOptions) { } export default async function (browserId) { - var processOptions = { arguments: browserId, psargs: 'aux' }; + var processOptions = { arguments: browserId, psargs: '-ef' }; var processList = OS.win ? await findProcessWin(processOptions) : await findProcess(processOptions); if (!processList.length) diff --git a/src/utils/temp-directory/cleanup-process/index.js b/src/utils/temp-directory/cleanup-process/index.js new file mode 100644 index 00000000000..4736f2bbc70 --- /dev/null +++ b/src/utils/temp-directory/cleanup-process/index.js @@ -0,0 +1,137 @@ +import { spawn } from 'child_process'; +import debug from 'debug'; +import promisifyEvent from 'promisify-event'; +import Promise from 'pinkie'; + + +const WORKER_PATH = require.resolve('./worker'); +const WORKER_STDIO_CONFIG = ['ignore', 'ignore', 'ignore', 'ipc']; + +const DEBUG_LOGGER = debug('testcafe:utils:temp-directory:cleanup-process'); + +class CleanupProcess { + constructor () { + this.worker = null; + this.initialized = false; + this.initPromise = Promise.resolve(void 0); + + this.messageCounter = 1; + + this.pendingResponses = {}; + } + + _sendMessage (id, msg) { + return new Promise((resolve, reject) => { + this.worker.send({ id, ...msg }, err => { + if (err) + reject(err); + else + resolve(); + }); + }); + } + + _onResponse (response) { + const pendingResponse = this.pendingResponses[response.id]; + + if (response.error) { + if (pendingResponse) + pendingResponse.control.reject(response.error); + else + this.pendingResponses[response.id] = Promise.reject(response.error); + } + else if (pendingResponse) + pendingResponse.control.resolve(); + else + this.pendingResponses[response.id] = Promise.resolve(); + } + + async _waitResponse (id) { + if (!this.pendingResponses[id]) { + const promiseControl = {}; + + this.pendingResponses[id] = new Promise((resolve, reject) => { + Object.assign(promiseControl, { resolve, reject }); + }); + + this.pendingResponses[id].control = promiseControl; + } + + try { + await this.pendingResponses[id]; + } + finally { + delete this.pendingResponses[id]; + } + } + + async _waitResponseForMessage (msg) { + const currentId = this.messageCounter; + + this.messageCounter++; + + await this._sendMessage(currentId, msg); + await this._waitResponse(currentId); + } + + init () { + this.initPromise = this.initPromise + .then(async initialized => { + if (initialized !== void 0) + return initialized; + + this.worker = spawn(process.argv[0], [WORKER_PATH], { detached: true, stdio: WORKER_STDIO_CONFIG }); + + this.worker.on('message', message => this._onResponse(message)); + + this.worker.unref(); + + try { + await Promise.race([ + this._waitResponse(0), + promisifyEvent(this.worker, 'error') + ]); + + this.initialized = true; + } + catch (e) { + DEBUG_LOGGER('Failed to start cleanup process'); + DEBUG_LOGGER(e); + + this.initialized = false; + } + + return this.initialized; + }); + + return this.initPromise; + } + + async addDirectory (path) { + if (!this.initialized) + return; + + try { + await this._waitResponseForMessage({ command: 'add', path }); + } + catch (e) { + DEBUG_LOGGER(`Failed to add the ${path} directory to cleanup process`); + DEBUG_LOGGER(e); + } + } + + async removeDirectory (path) { + if (!this.initialized) + return; + + try { + await this._waitResponseForMessage({ command: 'remove', path }); + } + catch (e) { + DEBUG_LOGGER(`Failed to remove the ${path} directory in cleanup process`); + DEBUG_LOGGER(e); + } + } +} + +export default new CleanupProcess(); diff --git a/src/utils/temp-directory/cleanup-process/worker.js b/src/utils/temp-directory/cleanup-process/worker.js new file mode 100644 index 00000000000..e574f3c58c7 --- /dev/null +++ b/src/utils/temp-directory/cleanup-process/worker.js @@ -0,0 +1,73 @@ +import path from 'path'; +import { inspect } from 'util'; +import del from 'del'; +import Promise from 'promise'; +import killBrowserProcess from '../../../browser/provider/utils/kill-browser-process'; + + +const DIRECTORIES_TO_CLEANUP = {}; + +function addDirectory (dirPath) { + if (!DIRECTORIES_TO_CLEANUP[dirPath]) + DIRECTORIES_TO_CLEANUP[dirPath] = {}; +} + +async function removeDirectory (dirPath) { + if (!DIRECTORIES_TO_CLEANUP[dirPath]) + return; + + let delPromise = DIRECTORIES_TO_CLEANUP[dirPath].delPromise; + + if (!delPromise) { + delPromise = killBrowserProcess(path.basename(dirPath)) + .then(() => del(dirPath, { force: true })); + + DIRECTORIES_TO_CLEANUP[dirPath].delPromise = delPromise; + } + + await DIRECTORIES_TO_CLEANUP[dirPath].delPromise; + + delete DIRECTORIES_TO_CLEANUP[dirPath].delPromise; +} + +async function dispatchCommand (message) { + switch (message.command) { + case 'add': + addDirectory(message.path); + return; + case 'remove': + addDirectory(message.path); + await removeDirectory(message.path); + return; + } +} + +function init () { + process.send({ id: 0 }); + + process.on('message', async message => { + let error = ''; + + try { + await dispatchCommand(message); + } + catch (e) { + error = inspect(e); + } + + process.send({ id: message.id, error }); + }); + + process.on('disconnect', async () => { + const removePromises = Object + .keys(DIRECTORIES_TO_CLEANUP) + .map(dirPath => removeDirectory(dirPath).catch(() => {})); + + await Promise.all(removePromises); + + process.exit(0); //eslint-disable-line no-process-exit + }); +} + +init(); + diff --git a/src/utils/temp-directory.js b/src/utils/temp-directory/index.js similarity index 81% rename from src/utils/temp-directory.js rename to src/utils/temp-directory/index.js index 8108b2858ce..fcbed26c1d9 100644 --- a/src/utils/temp-directory.js +++ b/src/utils/temp-directory/index.js @@ -4,7 +4,8 @@ import os from 'os'; import path from 'path'; import setupExitHook from 'async-exit-hook'; import tmp from 'tmp'; -import { ensureDir, readDir } from '../utils/promisified-functions'; +import cleanupProcess from './cleanup-process'; +import { ensureDir, readDir } from '../../utils/promisified-functions'; // NOTE: mutable for testing purposes @@ -24,9 +25,9 @@ class LockFile { this.path = path.join(dirPath, LOCKFILE_NAME); } - _openLockFile () { + _openLockFile ({ force = false } = {}) { try { - const fd = fs.openSync(this.path, 'wx'); + const fd = fs.openSync(this.path, force ? 'w' : 'wx'); fs.closeSync(fd); @@ -57,7 +58,13 @@ class LockFile { } init () { - return this._openLockFile() || this._isStaleLockFile(); + if (this._openLockFile()) + return true; + + if (this._isStaleLockFile()) + return this._openLockFile({ force: true }); + + return false; } dispose () { @@ -122,8 +129,8 @@ export default class TempDirectory { return tmpDir; } - static disposeDirectories () { - Object.values(USED_TEMP_DIRS).forEach(tmpDir => tmpDir.dispose()); + static disposeDirectoriesSync () { + Object.values(USED_TEMP_DIRS).forEach(tmpDir => tmpDir.disposeSync()); } async init () { @@ -140,15 +147,30 @@ export default class TempDirectory { DEBUG_LOGGER('Temp directory path: ', this.path); + await cleanupProcess.init(); + + await cleanupProcess.addDirectory(this.path); + USED_TEMP_DIRS[this.path] = this; } - dispose () { + disposeSync () { + if (!USED_TEMP_DIRS[this.path]) + return; + + this.lockFile.dispose(); + + delete USED_TEMP_DIRS[this.path]; + } + + async dispose () { if (!USED_TEMP_DIRS[this.path]) return; this.lockFile.dispose(); + await cleanupProcess.removeDirectory(this.path); + delete USED_TEMP_DIRS[this.path]; } @@ -164,4 +186,4 @@ export default class TempDirectory { } } -setupExitHook(TempDirectory.disposeDirectories); +setupExitHook(TempDirectory.disposeDirectoriesSync); diff --git a/test/server/util-test.js b/test/server/util-test.js index 371a3136ba6..b4a7ad81d6f 100644 --- a/test/server/util-test.js +++ b/test/server/util-test.js @@ -84,7 +84,7 @@ describe('Utils', () => { return del(TMP_ROOT); }); - it('Should reuse existing temp directories', function () { + it('Should reuse existing temp directories after synchronous disposal', function () { const tempDir1 = new TempDirectory(); const tempDir2 = new TempDirectory(); const tempDir3 = new TempDirectory(); @@ -92,12 +92,31 @@ describe('Utils', () => { return tempDir1 .init() .then(() => tempDir2.init()) - .then(() => tempDir1.dispose()) + .then(() => tempDir1.disposeSync()) .then(() => tempDir3.init()) .then(() => { const subDirs = fs.readdirSync(TempDirectory.TEMP_DIRECTORIES_ROOT); expect(subDirs.length).eql(2); + expect(tempDir3.path).eql(tempDir1.path); + }); + }); + + it('Should remove temp directories after asynchronous disposal', function () { + const tempDir = new TempDirectory(); + + return tempDir + .init() + .then(() => { + const subDirs = fs.readdirSync(TempDirectory.TEMP_DIRECTORIES_ROOT); + + expect(subDirs.length).eql(1); + }) + .then(() => tempDir.dispose()) + .then(() => { + const subDirs = fs.readdirSync(TempDirectory.TEMP_DIRECTORIES_ROOT); + + expect(subDirs.length).eql(0); }); }); }); From 16b8770bb40e8e7f741b23931af990562017769b Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Mon, 20 Aug 2018 11:03:14 +0300 Subject: [PATCH 4/5] Fix tests --- .../provider/built-in/chrome/local-chrome.js | 2 +- .../provider/built-in/chrome/runtime-info.js | 6 +- .../built-in/firefox/local-firefox.js | 2 +- .../utils/kill-browser-process.js | 2 +- .../cleanup-process/commands.js | 5 ++ .../temp-directory/cleanup-process/index.js | 13 ++-- .../temp-directory/cleanup-process/worker.js | 13 ++-- src/utils/temp-directory/index.js | 65 +----------------- src/utils/temp-directory/lockfile.js | 68 +++++++++++++++++++ test/server/util-test.js | 4 +- 10 files changed, 100 insertions(+), 80 deletions(-) rename src/{browser/provider => }/utils/kill-browser-process.js (95%) create mode 100644 src/utils/temp-directory/cleanup-process/commands.js create mode 100644 src/utils/temp-directory/lockfile.js diff --git a/src/browser/provider/built-in/chrome/local-chrome.js b/src/browser/provider/built-in/chrome/local-chrome.js index ca199035542..9a652375c6d 100644 --- a/src/browser/provider/built-in/chrome/local-chrome.js +++ b/src/browser/provider/built-in/chrome/local-chrome.js @@ -1,5 +1,5 @@ import browserTools from 'testcafe-browser-tools'; -import killBrowserProcess from '../../utils/kill-browser-process'; +import killBrowserProcess from '../../../../utils/kill-browser-process'; import BrowserStarter from '../../utils/browser-starter'; diff --git a/src/browser/provider/built-in/chrome/runtime-info.js b/src/browser/provider/built-in/chrome/runtime-info.js index 2f47c3de847..cb68ac36940 100644 --- a/src/browser/provider/built-in/chrome/runtime-info.js +++ b/src/browser/provider/built-in/chrome/runtime-info.js @@ -4,9 +4,9 @@ import createTempProfile from './create-temp-profile'; export default async function (proxyHostName, configString) { - var config = getConfig(configString); - var tempProfileDir = !config.userProfile ? await createTempProfile(proxyHostName, config) : null; - var cdpPort = config.cdpPort || (!config.userProfile ? await getFreePort() : null); + const config = getConfig(configString); + const tempProfileDir = !config.userProfile ? await createTempProfile(proxyHostName, config) : null; + const cdpPort = config.cdpPort || (!config.userProfile ? await getFreePort() : null); return { config, cdpPort, tempProfileDir }; } diff --git a/src/browser/provider/built-in/firefox/local-firefox.js b/src/browser/provider/built-in/firefox/local-firefox.js index efaafa6f8a0..a480fcdce11 100644 --- a/src/browser/provider/built-in/firefox/local-firefox.js +++ b/src/browser/provider/built-in/firefox/local-firefox.js @@ -1,6 +1,6 @@ import OS from 'os-family'; import browserTools from 'testcafe-browser-tools'; -import killBrowserProcess from '../../utils/kill-browser-process'; +import killBrowserProcess from '../../../../utils/kill-browser-process'; import BrowserStarter from '../../utils/browser-starter'; diff --git a/src/browser/provider/utils/kill-browser-process.js b/src/utils/kill-browser-process.js similarity index 95% rename from src/browser/provider/utils/kill-browser-process.js rename to src/utils/kill-browser-process.js index f875aa2de33..d1171619237 100644 --- a/src/browser/provider/utils/kill-browser-process.js +++ b/src/utils/kill-browser-process.js @@ -2,7 +2,7 @@ import { spawn } from 'child_process'; import OS from 'os-family'; import promisifyEvent from 'promisify-event'; import Promise from 'pinkie'; -import { findProcess, killProcess } from '../../../utils/promisified-functions'; +import { findProcess, killProcess } from './promisified-functions'; const BROWSER_CLOSING_TIMEOUT = 5; diff --git a/src/utils/temp-directory/cleanup-process/commands.js b/src/utils/temp-directory/cleanup-process/commands.js new file mode 100644 index 00000000000..619f22f5443 --- /dev/null +++ b/src/utils/temp-directory/cleanup-process/commands.js @@ -0,0 +1,5 @@ +export default { + init: 'init', + add: 'add', + remove: 'remove' +}; diff --git a/src/utils/temp-directory/cleanup-process/index.js b/src/utils/temp-directory/cleanup-process/index.js index 4736f2bbc70..26a2ef1857d 100644 --- a/src/utils/temp-directory/cleanup-process/index.js +++ b/src/utils/temp-directory/cleanup-process/index.js @@ -2,6 +2,7 @@ import { spawn } from 'child_process'; import debug from 'debug'; import promisifyEvent from 'promisify-event'; import Promise from 'pinkie'; +import COMMANDS from './commands'; const WORKER_PATH = require.resolve('./worker'); @@ -15,7 +16,7 @@ class CleanupProcess { this.initialized = false; this.initPromise = Promise.resolve(void 0); - this.messageCounter = 1; + this.messageCounter = 0; this.pendingResponses = {}; } @@ -88,10 +89,14 @@ class CleanupProcess { try { await Promise.race([ - this._waitResponse(0), + this._waitResponseForMessage({ command: COMMANDS.init }), promisifyEvent(this.worker, 'error') ]); + const channel = this.worker.channel || this.worker._channel; + + channel.unref(); + this.initialized = true; } catch (e) { @@ -112,7 +117,7 @@ class CleanupProcess { return; try { - await this._waitResponseForMessage({ command: 'add', path }); + await this._waitResponseForMessage({ command: COMMANDS.add, path }); } catch (e) { DEBUG_LOGGER(`Failed to add the ${path} directory to cleanup process`); @@ -125,7 +130,7 @@ class CleanupProcess { return; try { - await this._waitResponseForMessage({ command: 'remove', path }); + await this._waitResponseForMessage({ command: COMMANDS.remove, path }); } catch (e) { DEBUG_LOGGER(`Failed to remove the ${path} directory in cleanup process`); diff --git a/src/utils/temp-directory/cleanup-process/worker.js b/src/utils/temp-directory/cleanup-process/worker.js index e574f3c58c7..09ae7b31beb 100644 --- a/src/utils/temp-directory/cleanup-process/worker.js +++ b/src/utils/temp-directory/cleanup-process/worker.js @@ -2,7 +2,8 @@ import path from 'path'; import { inspect } from 'util'; import del from 'del'; import Promise from 'promise'; -import killBrowserProcess from '../../../browser/provider/utils/kill-browser-process'; +import killBrowserProcess from '../../kill-browser-process'; +import COMMANDS from './commands'; const DIRECTORIES_TO_CLEANUP = {}; @@ -32,19 +33,21 @@ async function removeDirectory (dirPath) { async function dispatchCommand (message) { switch (message.command) { - case 'add': + case COMMANDS.init: + return; + case COMMANDS.add: addDirectory(message.path); return; - case 'remove': + case COMMANDS.remove: addDirectory(message.path); await removeDirectory(message.path); return; + default: + return; } } function init () { - process.send({ id: 0 }); - process.on('message', async message => { let error = ''; diff --git a/src/utils/temp-directory/index.js b/src/utils/temp-directory/index.js index fcbed26c1d9..61cab9665d5 100644 --- a/src/utils/temp-directory/index.js +++ b/src/utils/temp-directory/index.js @@ -1,83 +1,22 @@ import debug from 'debug'; -import fs from 'fs'; import os from 'os'; import path from 'path'; import setupExitHook from 'async-exit-hook'; import tmp from 'tmp'; +import LockFile from './lockfile'; import cleanupProcess from './cleanup-process'; import { ensureDir, readDir } from '../../utils/promisified-functions'; // NOTE: mutable for testing purposes -let TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe-tmp'); +let TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe'); -const LOCKFILE_NAME = '.testcafe-lockfile'; const DEFAULT_NAME_PREFIX = 'tmp'; -const STALE_LOCKFILE_AGE = 2 * 24 * 60 * 60 * 1000; - const USED_TEMP_DIRS = {}; const DEBUG_LOGGER = debug('testcafe:utils:temp-directory'); -class LockFile { - constructor (dirPath) { - this.path = path.join(dirPath, LOCKFILE_NAME); - } - - _openLockFile ({ force = false } = {}) { - try { - const fd = fs.openSync(this.path, force ? 'w' : 'wx'); - - fs.closeSync(fd); - - return true; - } - catch (e) { - DEBUG_LOGGER('Failed to init lockfile ' + this.path); - DEBUG_LOGGER(e); - - return false; - } - } - - _isStaleLockFile () { - const currentMs = Date.now(); - - try { - const { mtimeMs } = fs.statSync(this.path); - - return currentMs - mtimeMs > STALE_LOCKFILE_AGE; - } - catch (e) { - DEBUG_LOGGER('Failed to check status of lockfile ' + this.path); - DEBUG_LOGGER(e); - - return false; - } - } - - init () { - if (this._openLockFile()) - return true; - - if (this._isStaleLockFile()) - return this._openLockFile({ force: true }); - - return false; - } - - dispose () { - try { - fs.unlinkSync(this.path); - } - catch (e) { - DEBUG_LOGGER('Failed to dispose lockfile ' + this.path); - DEBUG_LOGGER(e); - } - } -} - export default class TempDirectory { constructor (namePrefix) { this.namePrefix = namePrefix || DEFAULT_NAME_PREFIX; diff --git a/src/utils/temp-directory/lockfile.js b/src/utils/temp-directory/lockfile.js new file mode 100644 index 00000000000..838ae762240 --- /dev/null +++ b/src/utils/temp-directory/lockfile.js @@ -0,0 +1,68 @@ +import path from 'path'; +import debug from 'debug'; +import fs from 'fs'; + + +const LOCKFILE_NAME = '.testcafe-lockfile'; + +const STALE_LOCKFILE_AGE = 2 * 24 * 60 * 60 * 1000; + +const DEBUG_LOGGER = debug('testcafe:utils:temp-directory:lockfile'); + +export default class LockFile { + constructor (dirPath) { + this.path = path.join(dirPath, LOCKFILE_NAME); + } + + _open ({ force = false } = {}) { + try { + const fd = fs.openSync(this.path, force ? 'w' : 'wx'); + + fs.closeSync(fd); + + return true; + } + catch (e) { + DEBUG_LOGGER('Failed to init lockfile ' + this.path); + DEBUG_LOGGER(e); + + return false; + } + } + + _isStale () { + const currentMs = Date.now(); + + try { + const { mtimeMs } = fs.statSync(this.path); + + return currentMs - mtimeMs > STALE_LOCKFILE_AGE; + } + catch (e) { + DEBUG_LOGGER('Failed to check status of lockfile ' + this.path); + DEBUG_LOGGER(e); + + return false; + } + } + + init () { + if (this._open()) + return true; + + if (this._isStale()) + return this._open({ force: true }); + + return false; + } + + dispose () { + try { + fs.unlinkSync(this.path); + } + catch (e) { + DEBUG_LOGGER('Failed to dispose lockfile ' + this.path); + DEBUG_LOGGER(e); + } + } +} diff --git a/test/server/util-test.js b/test/server/util-test.js index b4a7ad81d6f..8c3cdda3470 100644 --- a/test/server/util-test.js +++ b/test/server/util-test.js @@ -8,8 +8,6 @@ const parseFileList = require('../../lib/utils/parse-file-list'); const TempDirectory = require('../../lib/utils/temp-directory'); -const TMP_ROOT = path.join(process.cwd(), '__tmp__'); - describe('Utils', () => { it('Correct File Path', () => { expect(correctFilePath('\\test')).eql('/test'); @@ -70,6 +68,8 @@ describe('Utils', () => { }); describe('Temp Directory', function () { + const TMP_ROOT = path.join(process.cwd(), '__tmp__'); + const savedTmpRoot = TempDirectory.TEMP_DIRECTORIES_ROOT; beforeEach(() => { From bb6d51341ea6c0700387f884bcd1bc265f542778 Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Thu, 23 Aug 2018 16:50:05 +0300 Subject: [PATCH 5/5] Fix remarks --- src/utils/promisified-functions.js | 2 + .../temp-directory/cleanup-process/index.js | 10 +--- .../temp-directory/cleanup-process/worker.js | 44 +++++++--------- src/utils/temp-directory/index.js | 52 +++++++------------ src/utils/temp-directory/lockfile.js | 12 ++--- test/server/util-test.js | 2 +- 6 files changed, 49 insertions(+), 73 deletions(-) diff --git a/src/utils/promisified-functions.js b/src/utils/promisified-functions.js index 0903d2f9039..5f0095a5281 100644 --- a/src/utils/promisified-functions.js +++ b/src/utils/promisified-functions.js @@ -16,3 +16,5 @@ export const findProcess = promisify(psNode.lookup); export const killProcess = promisify(psNode.kill); export const exec = promisify(childProcess.exec); + +export const sendMessageToChildProcess = promisify((process, ...args) => process.send(...args)); diff --git a/src/utils/temp-directory/cleanup-process/index.js b/src/utils/temp-directory/cleanup-process/index.js index 26a2ef1857d..a7b30893b78 100644 --- a/src/utils/temp-directory/cleanup-process/index.js +++ b/src/utils/temp-directory/cleanup-process/index.js @@ -2,6 +2,7 @@ import { spawn } from 'child_process'; import debug from 'debug'; import promisifyEvent from 'promisify-event'; import Promise from 'pinkie'; +import { sendMessageToChildProcess } from '../../promisified-functions'; import COMMANDS from './commands'; @@ -22,14 +23,7 @@ class CleanupProcess { } _sendMessage (id, msg) { - return new Promise((resolve, reject) => { - this.worker.send({ id, ...msg }, err => { - if (err) - reject(err); - else - resolve(); - }); - }); + return sendMessageToChildProcess(this.worker, { id, ...msg }); } _onResponse (response) { diff --git a/src/utils/temp-directory/cleanup-process/worker.js b/src/utils/temp-directory/cleanup-process/worker.js index 09ae7b31beb..987eb1191fc 100644 --- a/src/utils/temp-directory/cleanup-process/worker.js +++ b/src/utils/temp-directory/cleanup-process/worker.js @@ -1,7 +1,8 @@ import path from 'path'; import { inspect } from 'util'; import del from 'del'; -import Promise from 'promise'; +import Promise from 'pinkie'; +import { noop } from 'lodash'; import killBrowserProcess from '../../kill-browser-process'; import COMMANDS from './commands'; @@ -20,7 +21,7 @@ async function removeDirectory (dirPath) { let delPromise = DIRECTORIES_TO_CLEANUP[dirPath].delPromise; if (!delPromise) { - delPromise = killBrowserProcess(path.basename(dirPath)) + delPromise = killBrowserProcess(path.basename(dirPath)) .then(() => del(dirPath, { force: true })); DIRECTORIES_TO_CLEANUP[dirPath].delPromise = delPromise; @@ -42,35 +43,30 @@ async function dispatchCommand (message) { addDirectory(message.path); await removeDirectory(message.path); return; - default: - return; } } -function init () { - process.on('message', async message => { - let error = ''; +process.on('message', async message => { + let error = ''; - try { - await dispatchCommand(message); - } - catch (e) { - error = inspect(e); - } + try { + await dispatchCommand(message); + } + catch (e) { + error = inspect(e); + } - process.send({ id: message.id, error }); - }); + process.send({ id: message.id, error }); +}); - process.on('disconnect', async () => { - const removePromises = Object - .keys(DIRECTORIES_TO_CLEANUP) - .map(dirPath => removeDirectory(dirPath).catch(() => {})); +process.on('disconnect', async () => { + const removePromises = Object + .keys(DIRECTORIES_TO_CLEANUP) + .map(dirPath => removeDirectory(dirPath).catch(noop)); - await Promise.all(removePromises); + await Promise.all(removePromises); - process.exit(0); //eslint-disable-line no-process-exit - }); -} + process.exit(0); //eslint-disable-line no-process-exit +}); -init(); diff --git a/src/utils/temp-directory/index.js b/src/utils/temp-directory/index.js index 61cab9665d5..0df94003965 100644 --- a/src/utils/temp-directory/index.js +++ b/src/utils/temp-directory/index.js @@ -9,13 +9,10 @@ import { ensureDir, readDir } from '../../utils/promisified-functions'; // NOTE: mutable for testing purposes -let TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe'); - +const TESTCAFE_TMP_DIRS_ROOT = path.join(os.tmpdir(), 'testcafe'); const DEFAULT_NAME_PREFIX = 'tmp'; - -const USED_TEMP_DIRS = {}; - -const DEBUG_LOGGER = debug('testcafe:utils:temp-directory'); +const USED_TEMP_DIRS = {}; +const DEBUG_LOGGER = debug('testcafe:utils:temp-directory'); export default class TempDirectory { constructor (namePrefix) { @@ -26,7 +23,7 @@ export default class TempDirectory { } async _getTmpDirsList () { - const tmpDirNames = await readDir(TESTCAFE_TMP_DIRS_ROOT); + const tmpDirNames = await readDir(TempDirectory.TEMP_DIRECTORIES_ROOT); return tmpDirNames .filter(tmpDir => !USED_TEMP_DIRS[tmpDir]) @@ -35,7 +32,7 @@ export default class TempDirectory { async _findFreeTmpDir (tmpDirNames) { for (const tmpDirName of tmpDirNames) { - const tmpDirPath = path.join(TESTCAFE_TMP_DIRS_ROOT, tmpDirName); + const tmpDirPath = path.join(TempDirectory.TEMP_DIRECTORIES_ROOT, tmpDirName); const lockFile = new LockFile(tmpDirPath); @@ -51,7 +48,7 @@ export default class TempDirectory { } async _createNewTmpDir () { - this.path = tmp.tmpNameSync({ dir: TESTCAFE_TMP_DIRS_ROOT, prefix: this.namePrefix + '-' }); + this.path = tmp.tmpNameSync({ dir: TempDirectory.TEMP_DIRECTORIES_ROOT, prefix: this.namePrefix + '-' }); await ensureDir(this.path); @@ -60,6 +57,15 @@ export default class TempDirectory { this.lockFile.init(); } + _disposeSync () { + if (!USED_TEMP_DIRS[this.path]) + return; + + this.lockFile.dispose(); + + delete USED_TEMP_DIRS[this.path]; + } + static async createDirectory (prefix) { const tmpDir = new TempDirectory(prefix); @@ -69,11 +75,11 @@ export default class TempDirectory { } static disposeDirectoriesSync () { - Object.values(USED_TEMP_DIRS).forEach(tmpDir => tmpDir.disposeSync()); + Object.values(USED_TEMP_DIRS).forEach(tmpDir => tmpDir._disposeSync()); } async init () { - await ensureDir(TESTCAFE_TMP_DIRS_ROOT); + await ensureDir(TempDirectory.TEMP_DIRECTORIES_ROOT); const tmpDirNames = await this._getTmpDirsList(this.namePrefix); @@ -87,21 +93,11 @@ export default class TempDirectory { DEBUG_LOGGER('Temp directory path: ', this.path); await cleanupProcess.init(); - await cleanupProcess.addDirectory(this.path); USED_TEMP_DIRS[this.path] = this; } - disposeSync () { - if (!USED_TEMP_DIRS[this.path]) - return; - - this.lockFile.dispose(); - - delete USED_TEMP_DIRS[this.path]; - } - async dispose () { if (!USED_TEMP_DIRS[this.path]) return; @@ -112,17 +108,9 @@ export default class TempDirectory { delete USED_TEMP_DIRS[this.path]; } - - // NOTE: for testing purposes - static get TEMP_DIRECTORIES_ROOT () { - return TESTCAFE_TMP_DIRS_ROOT; - } - - static set TEMP_DIRECTORIES_ROOT (value) { - TESTCAFE_TMP_DIRS_ROOT = value; - - return value; - } } +// NOTE: exposed for testing purposes +TempDirectory.TEMP_DIRECTORIES_ROOT = TESTCAFE_TMP_DIRS_ROOT; + setupExitHook(TempDirectory.disposeDirectoriesSync); diff --git a/src/utils/temp-directory/lockfile.js b/src/utils/temp-directory/lockfile.js index 838ae762240..fb9857a20ec 100644 --- a/src/utils/temp-directory/lockfile.js +++ b/src/utils/temp-directory/lockfile.js @@ -3,22 +3,18 @@ import debug from 'debug'; import fs from 'fs'; -const LOCKFILE_NAME = '.testcafe-lockfile'; - +const LOCKFILE_NAME = '.testcafe-lockfile'; const STALE_LOCKFILE_AGE = 2 * 24 * 60 * 60 * 1000; - -const DEBUG_LOGGER = debug('testcafe:utils:temp-directory:lockfile'); +const DEBUG_LOGGER = debug('testcafe:utils:temp-directory:lockfile'); export default class LockFile { constructor (dirPath) { - this.path = path.join(dirPath, LOCKFILE_NAME); + this.path = path.join(dirPath, LOCKFILE_NAME); } _open ({ force = false } = {}) { try { - const fd = fs.openSync(this.path, force ? 'w' : 'wx'); - - fs.closeSync(fd); + fs.writeFileSync(this.path, '', { flag: force ? 'w' : 'wx' }); return true; } diff --git a/test/server/util-test.js b/test/server/util-test.js index 8c3cdda3470..097e9ce788e 100644 --- a/test/server/util-test.js +++ b/test/server/util-test.js @@ -92,7 +92,7 @@ describe('Utils', () => { return tempDir1 .init() .then(() => tempDir2.init()) - .then(() => tempDir1.disposeSync()) + .then(() => tempDir1._disposeSync()) .then(() => tempDir3.init()) .then(() => { const subDirs = fs.readdirSync(TempDirectory.TEMP_DIRECTORIES_ROOT);