From c10da7a3c1be6918fd2d84c36d39db182c0486b5 Mon Sep 17 00:00:00 2001 From: Andrey Chalkin Date: Thu, 19 Sep 2024 08:23:37 +0200 Subject: [PATCH] fix: stop command not working on Windows OS (#313) --- .github/workflows/ci.yml | 14 ++--- lib/daemon.js | 2 +- lib/launcher.js | 22 +++++++- lib/launcher.test.js | 110 ++++++++++++++++++++++++++++++++++++++- lib/service.js | 8 ++- lib/service.test.js | 12 ++++- test/test.integration.js | 4 +- 7 files changed, 159 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f935fb..c39f403 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,31 +10,33 @@ on: jobs: build: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: matrix: + os: [ubuntu-latest, windows-latest] node-version: ['18.x', '20.x', '22.x'] steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} cache: 'npm' - name: Install run: npm ci - name: Lint - if: matrix.node-version == '22.x' + if: matrix.os == 'ubuntu-latest' && matrix.node-version == '22.x' run: npm run lint - name: Types - if: matrix.node-version == '22.x' + if: matrix.os == 'ubuntu-latest' && matrix.node-version == '22.x' run: node_modules/.bin/tsc - name: Prettier - if: matrix.node-version == '22.x' + if: matrix.os == 'ubuntu-latest' && matrix.node-version == '22.x' run: npm run prettier:check - name: Test run: npm test - name: Integration Tests + if: matrix.os == 'ubuntu-latest' run: npm run test:integration diff --git a/lib/daemon.js b/lib/daemon.js index 2e0f817..8d84f91 100644 --- a/lib/daemon.js +++ b/lib/daemon.js @@ -25,7 +25,7 @@ let idle_timeout = null; /** @type {fs.FSWatcher | null} */ let watcher = null; -let service = createService(resolver, token); +let service = createService(resolver, token, shutdown); if (idle) { service = watchConnection(service, idle * 60000); } diff --git a/lib/launcher.js b/lib/launcher.js index bc1d714..6383f41 100644 --- a/lib/launcher.js +++ b/lib/launcher.js @@ -1,4 +1,6 @@ import fs from 'node:fs'; +import net from 'node:net'; +import os from 'node:os'; import child_process from 'node:child_process'; import { loadConfig, removeConfig } from './config.js'; @@ -46,7 +48,7 @@ export async function launchDaemon(resolver, hash) { */ export async function stopDaemon(resolver, config) { try { - process.kill(config.pid, 'SIGTERM'); + await platformAwareStopDaemon(config); } catch (err) { console.error(`eslint_d: ${err} - removing config`); await removeConfig(resolver); @@ -55,6 +57,24 @@ export async function stopDaemon(resolver, config) { await waitForConfig(resolver.base); } +/** + * @param {Config} config + * @returns {Promise} + */ +function platformAwareStopDaemon(config) { + if (os.platform() === 'win32') { + return new Promise((resolve, reject) => { + const socket = net.connect(config.port, '127.0.0.1'); + socket.once('error', reject); + socket.write(JSON.stringify([config.token, 'ESLINT_D_STOP'])); + socket.end(() => resolve(undefined)); + }); + } + + process.kill(config.pid, 'SIGTERM'); + return Promise.resolve(); +} + /** * @returns {string} */ diff --git a/lib/launcher.test.js b/lib/launcher.test.js index dafaef9..2040480 100644 --- a/lib/launcher.test.js +++ b/lib/launcher.test.js @@ -1,10 +1,13 @@ import fs from 'node:fs'; import fs_promises from 'node:fs/promises'; +import os from 'node:os'; +import net from 'node:net'; import child_process from 'node:child_process'; import EventEmitter from 'node:events'; import { assert, refute, sinon } from '@sinonjs/referee-sinon'; import { createResolver } from './resolver.js'; import { launchDaemon, stopDaemon } from './launcher.js'; +import { strictEqual } from 'node:assert'; describe('lib/launcher', () => { const resolver = createResolver(); @@ -186,7 +189,11 @@ describe('lib/launcher', () => { }); }); - context('stopDaemon', () => { + context('unix: stopDaemon', () => { + if (os.platform() === 'win32') { + return; + } + const config = { token: 'token', port: 123, pid: 456, hash: 'hash' }; beforeEach(() => { @@ -213,6 +220,8 @@ describe('lib/launcher', () => { it('waits for the config to be removed and resolves', async () => { const promise = stopDaemon(resolver, config); + await new Promise((resolve) => setTimeout(resolve)); + assert.calledOnceWith(fs.watch, resolver.base); watcher.emit('change', 'rename', '.eslint_d'); @@ -246,4 +255,103 @@ describe('lib/launcher', () => { }); }); }); + + context('win32: stopDaemon', () => { + if (os.platform() !== 'win32') { + return; + } + + let server; + let config; + const sockets = []; + beforeEach((done) => { + server = net.createServer(); + server.on('connection', (socket) => sockets.push(socket)); + server.listen(0, '127.0.0.1', () => { + const port = server.address()?.['port']; + config = { token: 'token', port, pid: 456, hash: 'hash' }; + done(); + }); + }); + + afterEach((done) => { + sockets.forEach((socket) => socket.destroy()); + server.close(done); + }); + + beforeEach(() => { + sinon.replace(fs_promises, 'unlink', sinon.fake.resolves()); + }); + + context('without exception', () => { + it('send stop command to server listening on port from config', (done) => { + stopDaemon(resolver, config); + + let data = ''; + server.once('connection', (socket) => { + socket + .on('data', (buf) => { + data += buf.toString(); + }) + .on('end', () => { + strictEqual(data, '["token","ESLINT_D_STOP"]'); + done(); + }); + }); + }); + + it('does not remove the config', async () => { + stopDaemon(resolver, config); + + await new Promise((resolve) => { + server.on('connection', (socket) => { + socket.on('data', () => {}).on('end', () => resolve('')); + }); + }); + + refute.called(fs_promises.unlink); + }); + + it('waits for the config to be removed and resolves', async () => { + const promise = stopDaemon(resolver, config); + + await new Promise((resolve) => { + server.on('connection', (socket) => { + socket.on('data', () => {}).on('end', () => resolve('')); + }); + }); + + assert.calledOnceWith(fs.watch, resolver.base); + + watcher.emit('change', 'rename', '.eslint_d'); + + assert.calledOnceWith(watcher.close); + await assert.resolves(promise, undefined); + }); + }); + + context('with exception', () => { + beforeEach(() => { + const error = new Error('kill error'); + sinon.replace(net, 'connect', sinon.fake.throws(error)); + }); + + it('logs an error and removes the config file', async () => { + const promise = stopDaemon(resolver, config); + + await assert.resolves(promise, undefined); + assert.calledOnceWith( + console.error, + 'eslint_d: Error: kill error - removing config' + ); + assert.calledOnceWith(fs_promises.unlink, `${resolver.base}/.eslint_d`); + }); + + it('does not watch for the config file', () => { + stopDaemon(resolver, config); + + refute.called(fs.watch); + }); + }); + }); }); diff --git a/lib/service.js b/lib/service.js index cfbff4e..622c674 100644 --- a/lib/service.js +++ b/lib/service.js @@ -11,9 +11,10 @@ const stderr_write = process.stderr.write; /** * @param {Resolver} resolver * @param {string} token + * @param {() => void} shutdown * @returns {function(Socket): void} con */ -export function createService(resolver, token) { +export function createService(resolver, token, shutdown) { const eslint = resolver.require(`${resolver.base}/lib/cli.js`); const chalk = createRequire(resolver.base)('chalk'); @@ -44,6 +45,11 @@ export function createService(resolver, token) { con.end(); return; } + if (color_level === 'ESLINT_D_STOP') { + shutdown(); + con.end(); + return; + } chalk.level = color_level; process.chdir(cwd); diff --git a/lib/service.test.js b/lib/service.test.js index 4650174..2e31bf5 100644 --- a/lib/service.test.js +++ b/lib/service.test.js @@ -14,15 +14,19 @@ describe('lib/service', () => { const eslint = resolver.require(`${resolver.base}/lib/cli.js`); const chalk = resolver.require('chalk'); const token = 'token'; + let shutdown_promise; let eslint_promise; let service; let con; beforeEach(() => { eslint_promise = sinon.promise(); + shutdown_promise = sinon.promise(); sinon.replace(eslint, 'execute', sinon.fake.returns(eslint_promise)); sinon.replace(process, 'chdir', sinon.fake()); - service = createService(resolver, token); + service = createService(resolver, token, () => + shutdown_promise.resolve() + ); chalk.level = '-'; con = new Socket({ allowHalfOpen: true }); sinon.replace(con, 'write', sinon.fake()); @@ -150,5 +154,11 @@ describe('lib/service', () => { assert.calledOnceWith(con.write, 'Error: Ouch!'); assert.calledOnceWith(con.end, 'EXIT001'); }); + + it('shutdown daemon if ESLINT_D_STOP received', async () => { + send(token, '"ESLINT_D_STOP"', '/', []); + + await shutdown_promise; + }); }); }); diff --git a/test/test.integration.js b/test/test.integration.js index 6f645ed..cfe858f 100644 --- a/test/test.integration.js +++ b/test/test.integration.js @@ -32,7 +32,7 @@ describe('integration tests', () => { return new Promise((resolve) => { const child = child_process.exec( - `${bin} ${args}`, + `"${process.argv0}" ${bin} ${args}`, { cwd }, (error, stdout, stderr) => resolve({ error, stdout, stderr }) ); @@ -151,7 +151,7 @@ describe('integration tests', () => { it('fail.js', async () => { const { error, stdout, stderr } = await run('../fail.js', { cwd }); - assert.match(stdout, '/test/fixture/fail.js'); + assert.match(stdout, path.normalize('/test/fixture/fail.js')); assert.match(stdout, 'Strings must use singlequote'); refute.isNull(error); assert.equals(error?.['code'], 1);