Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove bluebird #436

Closed
wants to merge 10 commits into from
7 changes: 7 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
"files": "test/**/*.js",
"rules": {
"func-names": "off",
"promise/no-native": "off"
}
},
{
"files": "lib/**/*.js",
"rules": {
"promise/no-native": "off"
talentlessguy marked this conversation as resolved.
Show resolved Hide resolved
"@typescript-eslint/no-var-requires": "off"
}
},
Expand Down
3 changes: 1 addition & 2 deletions lib/exec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { spawn } from 'child_process';
import { quote } from 'shell-quote';
import B from 'bluebird';
import _ from 'lodash';
import { formatEnoent } from './helpers';
import { CircularBuffer, MAX_BUFFER_SIZE } from './circular-buffer';
Expand Down Expand Up @@ -37,7 +36,7 @@ async function exec (cmd, args = [], originalOpts = /** @type {T} */({})) {
const isBuffer = Boolean(opts.isBuffer);

// this is an async function, so return a promise
return await new B((resolve, reject) => {
return await new Promise((resolve, reject) => {
// spawn the child process with options; we don't currently expose any of
// the other 'spawn' options through the API
const proc = spawn(cmd, args, {cwd: opts.cwd, env: opts.env, shell: opts.shell});
Expand Down
11 changes: 5 additions & 6 deletions lib/subprocess.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { spawn } from 'child_process';
import events from 'events';
import * as events from 'events';
const { EventEmitter } = events;
import B from 'bluebird';
import { quote } from 'shell-quote';
import _ from 'lodash';
import { formatEnoent } from './helpers';
Expand Down Expand Up @@ -122,7 +121,7 @@ class SubProcess extends EventEmitter {
}

// return a promise so we can wrap the async behavior
return await new B((resolve, reject) => {
return await /** @type {Promise<void>} */(new Promise((resolve, reject) => {
// actually spawn the subproc
this.proc = spawn(this.cmd, this.args, this.opts);

Expand Down Expand Up @@ -224,7 +223,7 @@ class SubProcess extends EventEmitter {
`(cmd: '${this.rep}')`));
}, timeoutMs);
}
}).finally(() => {
})).finally(() => {
if (detach && this.proc) {
this.proc.unref();
}
Expand All @@ -249,7 +248,7 @@ class SubProcess extends EventEmitter {
if (!this.isRunning) {
throw new Error(`Can't stop process; it's not currently running (cmd: '${this.rep}')`);
}
return await new B((resolve, reject) => {
return await new Promise((resolve, reject) => {
this.proc?.on('close', resolve);
this.expectingExit = true;
this.proc?.kill(signal);
Expand All @@ -266,7 +265,7 @@ class SubProcess extends EventEmitter {
throw new Error(`Cannot join process; it is not currently running (cmd: '${this.rep}')`);
}

return await new B((resolve, reject) => {
return await new Promise((resolve, reject) => {
this.proc?.on('exit', (code) => {
if (code !== null && allowedExitCodes.indexOf(code) === -1) {
reject(new Error(`Process ended with exitcode ${code} (cmd: '${this.rep}')`));
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"singleQuote": true
},
"dependencies": {
"bluebird": "^3.7.2",
"lodash": "^4.17.21",
"shell-quote": "^1.8.1",
"source-map-support": "^0.x"
Expand All @@ -58,7 +57,7 @@
"@appium/types": "^0.x",
"@semantic-release/changelog": "^6.0.3",
"@semantic-release/git": "^10.0.1",
"@types/bluebird": "^3.5.42",
"@types/chai-as-promised": "^7.1.8",
talentlessguy marked this conversation as resolved.
Show resolved Hide resolved
"@types/lodash": "^4.14.202",
"@types/node": "^20.10.4",
"@types/shell-quote": "^1.7.5",
Expand All @@ -70,8 +69,9 @@
"mocha": "^10.2.0",
"prettier": "^3.1.0",
"semantic-release": "^24.0.0",
"typescript": "^5.4.1",
"ts-node": "^10.9.1"
"sinon": "^18.0.0",
talentlessguy marked this conversation as resolved.
Show resolved Hide resolved
"ts-node": "^10.9.1",
"typescript": "^5.4.1"
},
"engines": {
"node": "^16.13.0 || >=18.0.0",
Expand Down
46 changes: 24 additions & 22 deletions test/subproc-specs.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import B from 'bluebird';
import path from 'path';
import * as path from 'path';
import {exec, SubProcess} from '../lib';
import {getFixture} from './helpers';

// Windows doesn't understand SIGHUP
const stopSignal = process.platform === 'win32' ? 'SIGTERM' : 'SIGHUP';

/**
* @param {number} ms
*/
function delay(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}


describe('SubProcess', function () {
let chai;
let chaiAsPromised;
Expand Down Expand Up @@ -65,7 +72,7 @@ describe('SubProcess', function () {
lines = lines.concat(newLines);
});
await subproc.start(0);
await B.delay(50);
await delay(50);
lines.should.include('bad_exit.sh');
lines.should.contain('bigbuffer.js');
lines.should.contain('echo.sh');
Expand Down Expand Up @@ -116,7 +123,7 @@ describe('SubProcess', function () {
});
await s.start(0);
hasData.should.be.false;
await B.delay(1200);
await delay(1200);
hasData.should.be.true;
});
it('should fail even with a start timeout of 0 when command is bad', async function () {
Expand Down Expand Up @@ -154,14 +161,17 @@ describe('SubProcess', function () {
});

describe('listening for data', function () {
/**
* @type {SubProcess}
*/
let subproc;
afterEach(async function () {
try {
await subproc.stop();
} catch (ign) {}
});
it('should get output as params', async function () {
await new B(async (resolve, reject) => {
await /** @type {Promise<void>} */(new Promise((resolve, reject) => {
subproc = new SubProcess(getFixture('sleepyproc'), [
'ls',
path.resolve(__dirname),
Expand All @@ -173,21 +183,17 @@ describe('SubProcess', function () {
resolve();
}
});
await subproc.start();
}).should.eventually.not.be.rejected;
return subproc.start();
})).should.eventually.not.be.rejected;
});
it('should get output as params', async function () {
await new B(async (resolve, reject) => {
subproc = new SubProcess(getFixture('echo'), ['foo', 'bar']);
subproc.on('output', (stdout, stderr) => {
if (stderr && stderr.indexOf('bar') === -1) {
reject();
} else {
resolve();
throw new Error();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change would create an uncaught exception failure

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried flipping the if statement and it stops the test, so the exception doesn't hang

});
await subproc.start();
});
});

it('should get output by lines', async function () {
Expand All @@ -197,7 +203,7 @@ describe('SubProcess', function () {
lines = lines.concat(newLines);
});
await subproc.start(0);
await B.delay(50);
await delay(50);
lines.should.eql([
'circular-buffer-specs.js',
'exec-specs.js',
Expand All @@ -210,22 +216,18 @@ describe('SubProcess', function () {

describe('#stop', function () {
it('should send the right signal to stop a proc', async function () {
return await new B(async (resolve, reject) => {
let subproc = new SubProcess('tail', ['-f', path.resolve(__filename)]);
const subproc = new SubProcess('tail', ['-f', path.resolve(__filename)]);

await subproc.start();

subproc.on('exit', (code, signal) => {
try {
signal.should.equal(stopSignal);
resolve();
} catch (e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this change works as expected in case of a failure

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the old code it would reject a promise which would return an exception and fail the test. now it just throws directly and also stops the test, so there's virtually no difference:

 1) SubProcess
       #stop
         should send the right signal to stop a proc:
     Uncaught 
  Error
      at SubProcess.<anonymous> (test/subproc-specs.js:225:19)
      at SubProcess.emit (node:events:520:28)
      at SubProcess.emit (node:domain:488:12)
      at ChildProcess.<anonymous> (lib/subprocess.js:193:14)
      at ChildProcess.emit (node:events:520:28)
      at ChildProcess.emit (node:domain:488:12)
      at Process.ChildProcess._handle.onexit (node:internal/child_process:294:12)

reject(e);
}
});

await subproc.stop(stopSignal);
});
});


it('should time out if stop doesnt complete fast enough', async function () {
let subproc = new SubProcess(getFixture('traphup'), [
'tail',
Expand All @@ -251,7 +253,7 @@ describe('SubProcess', function () {
let subproc = new SubProcess('ls');
await subproc.stop().should.eventually.be.rejectedWith(/Can't stop/);
await subproc.start();
await B.delay(10);
await delay(10);
await subproc.stop().should.eventually.be.rejectedWith(/Can't stop/);
});
});
Expand Down
Loading