From 5f61c8e456bf77803a583605c2a5f70b97b503ad Mon Sep 17 00:00:00 2001 From: Damien Cassou Date: Tue, 30 Jul 2024 17:42:49 +0200 Subject: [PATCH 1/4] Add unit test to forwarder Just so I feel safer with the next refactoring. --- lib/forwarder.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/forwarder.test.js b/lib/forwarder.test.js index 0a56eff..6eb6119 100644 --- a/lib/forwarder.test.js +++ b/lib/forwarder.test.js @@ -138,6 +138,25 @@ describe('lib/forwarder', () => { refute.called(console.error); }); + it('handles EXIT1 inside response', () => { + const chunks = ['response EXIT1', ' from daemonEXIT1']; + sinon.replace( + socket, + 'read', + sinon.fake(() => (chunks.length ? chunks.shift() : null)) + ); + sinon.replace(process.stdout, 'write', sinon.fake()); + + forwardToDaemon(resolver, config); + socket.on.firstCall.callback(); // readable + socket.on.secondCall.callback(); // end + + assert.calledWith(process.stdout.write, 'response '); + assert.calledWith(process.stdout.write, 'EXIT1 from daemon'); + assert.equals(process.exitCode, 1); + refute.called(console.error); + }); + it('logs error and sets exitCode to 1 if response does not end with EXIT marker', () => { const chunks = ['response from daemon']; sinon.replace( From d6fc9990e93bff764c799ee1c7dccbc80e3737a5 Mon Sep 17 00:00:00 2001 From: Damien Cassou Date: Wed, 31 Jul 2024 21:13:58 +0200 Subject: [PATCH 2/4] Improve a unit test of forwarder We want to check what is written to stdout, not the implementation details of which event handler writes what. --- lib/forwarder.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/forwarder.test.js b/lib/forwarder.test.js index 6eb6119..89ec919 100644 --- a/lib/forwarder.test.js +++ b/lib/forwarder.test.js @@ -85,7 +85,7 @@ describe('lib/forwarder', () => { assert.calledWith(socket.write, text); }); - it('forwards socket response to stdout, except for the last 5 characters', () => { + it('forwards socket response to stdout', () => { const chunks = ['response ', 'from daemon']; sinon.replace( socket, @@ -96,10 +96,12 @@ describe('lib/forwarder', () => { forwardToDaemon(resolver, config); socket.on.firstCall.callback(); // readable + socket.on.secondCall.callback(); // end - assert.calledTwice(process.stdout.write); + assert.calledThrice(process.stdout.write); assert.calledWith(process.stdout.write, 'resp'); assert.calledWith(process.stdout.write, 'onse from d'); + assert.calledWith(process.stdout.write, 'aemon'); }); it('handles EXIT0 from response', () => { From d803e5ac6e433b4cc13bc19a2c19c99500c15f60 Mon Sep 17 00:00:00 2001 From: Damien Cassou Date: Wed, 31 Jul 2024 20:54:47 +0200 Subject: [PATCH 3/4] Refactor forwarder to remove a limitation in the size of the error code The previous implementation only worked for exit codes of one character and had repeated magic numbers. --- lib/forwarder.js | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/forwarder.js b/lib/forwarder.js index 1f60912..c5d2f88 100644 --- a/lib/forwarder.js +++ b/lib/forwarder.js @@ -7,6 +7,18 @@ import { removeConfig } from '../lib/config.js'; * @import { Resolver} from '../lib/resolver.js' */ +/* Maximum number of characters of all eslint exit codes */ +const EXIT_CODE_MAX_LENGTH = 1; + +/* The string sent by our daemon upon termination */ +const DAEMON_EXIT_TOKEN = 'EXIT'; + +const DAEMON_TERMINATION_CODE_MAX_LENGTH = + DAEMON_EXIT_TOKEN.length + EXIT_CODE_MAX_LENGTH; +const DAEMON_TERMINATION_CODE_REGEXP = new RegExp( + `${DAEMON_EXIT_TOKEN}([0-9]{1,${EXIT_CODE_MAX_LENGTH}})` +); + /** * @param {Resolver} resolver * @param {Config} config @@ -36,19 +48,38 @@ export async function forwardToDaemon(resolver, config) { let chunk = ''; while ((chunk = socket.read()) !== null) { content += chunk; - if (content.length > 5) { - process.stdout.write(content.substring(0, content.length - 5)); - content = content.substring(content.length - 5); + if (content.length > DAEMON_TERMINATION_CODE_MAX_LENGTH) { + const message_length = + content.length - DAEMON_TERMINATION_CODE_MAX_LENGTH; + // write everything we are sure doesn't contain the termination code: + process.stdout.write(content.substring(0, message_length)); + // keep only what we haven't written yet: + content = content.substring(message_length); } } }) .on('end', () => { - if (content.startsWith('EXIT')) { - process.exitCode = Number(content.slice(4)); - } else { + // search the end of 'content' for the termination code: + const endOfContent = content.slice(-DAEMON_TERMINATION_CODE_MAX_LENGTH); + const match = endOfContent.match(DAEMON_TERMINATION_CODE_REGEXP); + + if (!match) { process.stdout.write(content); console.error('eslint_d: unexpected response'); process.exitCode = 1; + return; + } + + // write everything but the termination code: + content = content.slice( + 0, + -DAEMON_TERMINATION_CODE_MAX_LENGTH + (match.index || 0) + ); + + process.exitCode = Number(match[1]); + + if (content) { + process.stdout.write(content); } }) .on('error', async (err) => { From 756f57c0beb153ef1c4ae2796b7f7a602a0e4197 Mon Sep 17 00:00:00 2001 From: Damien Cassou Date: Wed, 31 Jul 2024 20:57:02 +0200 Subject: [PATCH 4/4] Allow eslint exit code to have a length of 2 digits --- lib/forwarder.js | 2 +- lib/forwarder.test.js | 39 ++++++++++++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/forwarder.js b/lib/forwarder.js index c5d2f88..72dc19f 100644 --- a/lib/forwarder.js +++ b/lib/forwarder.js @@ -8,7 +8,7 @@ import { removeConfig } from '../lib/config.js'; */ /* Maximum number of characters of all eslint exit codes */ -const EXIT_CODE_MAX_LENGTH = 1; +const EXIT_CODE_MAX_LENGTH = 2; /* The string sent by our daemon upon termination */ const DAEMON_EXIT_TOKEN = 'EXIT'; diff --git a/lib/forwarder.test.js b/lib/forwarder.test.js index 89ec919..7cf4b6e 100644 --- a/lib/forwarder.test.js +++ b/lib/forwarder.test.js @@ -99,9 +99,9 @@ describe('lib/forwarder', () => { socket.on.secondCall.callback(); // end assert.calledThrice(process.stdout.write); - assert.calledWith(process.stdout.write, 'resp'); - assert.calledWith(process.stdout.write, 'onse from d'); - assert.calledWith(process.stdout.write, 'aemon'); + assert.calledWith(process.stdout.write, 'res'); + assert.calledWith(process.stdout.write, 'ponse from '); + assert.calledWith(process.stdout.write, 'daemon'); }); it('handles EXIT0 from response', () => { @@ -117,7 +117,8 @@ describe('lib/forwarder', () => { socket.on.firstCall.callback(); // readable socket.on.secondCall.callback(); // end - assert.calledOnceWith(process.stdout.write, 'response from daemon'); + assert.calledWith(process.stdout.write, 'response from daemo'); + assert.calledWith(process.stdout.write, 'n'); assert.equals(process.exitCode, 0); refute.called(console.error); }); @@ -135,11 +136,30 @@ describe('lib/forwarder', () => { socket.on.firstCall.callback(); // readable socket.on.secondCall.callback(); // end - assert.calledWith(process.stdout.write, 'response from daemon'); + assert.calledWith(process.stdout.write, 'response from daemo'); + assert.calledWith(process.stdout.write, 'n'); assert.equals(process.exitCode, 1); refute.called(console.error); }); + it('handles EXIT12 from response', () => { + const chunks = ['response from daemonEXIT12']; + sinon.replace( + socket, + 'read', + sinon.fake(() => (chunks.length ? chunks.shift() : null)) + ); + sinon.replace(process.stdout, 'write', sinon.fake()); + + forwardToDaemon(resolver, config); + socket.on.firstCall.callback(); // readable + socket.on.secondCall.callback(); // end + + assert.calledWith(process.stdout.write, 'response from daemon'); + assert.equals(process.exitCode, 12); + refute.called(console.error); + }); + it('handles EXIT1 inside response', () => { const chunks = ['response EXIT1', ' from daemonEXIT1']; sinon.replace( @@ -153,8 +173,9 @@ describe('lib/forwarder', () => { socket.on.firstCall.callback(); // readable socket.on.secondCall.callback(); // end - assert.calledWith(process.stdout.write, 'response '); - assert.calledWith(process.stdout.write, 'EXIT1 from daemon'); + assert.calledWith(process.stdout.write, 'response'); + assert.calledWith(process.stdout.write, ' EXIT1 from daemo'); + assert.calledWith(process.stdout.write, 'n'); assert.equals(process.exitCode, 1); refute.called(console.error); }); @@ -172,8 +193,8 @@ describe('lib/forwarder', () => { socket.on.firstCall.callback(); // readable socket.on.secondCall.callback(); // end - assert.calledWith(process.stdout.write, 'response from d'); - assert.calledWith(process.stdout.write, 'aemon'); + assert.calledWith(process.stdout.write, 'response from '); + assert.calledWith(process.stdout.write, 'daemon'); assert.equals(process.exitCode, 1); assert.calledOnceWith(console.error, 'eslint_d: unexpected response'); });