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

test: use ES6 classes instead of util.inherits #16938

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test: use ES6 classes instead of util.inherits
  • Loading branch information
tniessen committed Nov 10, 2017
commit c85977c84b819db013cd63b8ab9a3844db8684b3
11 changes: 5 additions & 6 deletions test/parallel/test-crypto-lazy-transform-writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ if (!common.hasCrypto)
const assert = require('assert');
const crypto = require('crypto');
const Stream = require('stream');
const util = require('util');

const hasher1 = crypto.createHash('sha256');
const hasher2 = crypto.createHash('sha256');
Expand All @@ -18,12 +17,12 @@ hasher1.end();

const expected = hasher1.read().toString('hex');

function OldStream() {
Stream.call(this);

this.readable = true;
class OldStream extends Stream {
constructor() {
super();
this.readable = true;
}
}
util.inherits(OldStream, Stream);

const stream = new OldStream();

Expand Down
26 changes: 13 additions & 13 deletions test/parallel/test-crypto-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,24 @@ if (!common.hasCrypto)

const assert = require('assert');
const stream = require('stream');
const util = require('util');
const crypto = require('crypto');

// Small stream to buffer converter
function Stream2buffer(callback) {
stream.Writable.call(this);
class Stream2buffer extends stream.Writable {
constructor(callback) {
super();

this._buffers = [];
this.once('finish', function() {
callback(null, Buffer.concat(this._buffers));
});
}
util.inherits(Stream2buffer, stream.Writable);
this._buffers = [];
this.once('finish', function() {
callback(null, Buffer.concat(this._buffers));
});
}

Stream2buffer.prototype._write = function(data, encodeing, done) {
this._buffers.push(data);
return done(null);
};
_write(data, encodeing, done) {
this._buffers.push(data);
return done(null);
}
}

Copy link
Member

Choose a reason for hiding this comment

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

(while we are here) how about eclosing the class definition inside the common.hasFipsCrypto block as it is used only if the conditional is met?

Copy link
Member

Choose a reason for hiding this comment

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

I mean enclosing

if (!common.hasFipsCrypto) {
// Create an md5 hash of "Hallo world"
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-event-emitter-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@
require('../common');
const assert = require('assert');
const events = require('events');
const util = require('util');

function listener() {}
function listener2() {}
class TestStream { constructor() { } }
util.inherits(TestStream, events.EventEmitter);
class TestStream extends events.EventEmitter {}

Copy link
Member

Choose a reason for hiding this comment

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

same comment as above - confining the scope into the block where it is used.

{
const ee = new events.EventEmitter();
Expand Down
46 changes: 21 additions & 25 deletions test/parallel/test-http-client-read-in-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,34 @@
require('../common');
const net = require('net');
const http = require('http');
const util = require('util');

function Agent() {
http.Agent.call(this);
}
util.inherits(Agent, http.Agent);

Agent.prototype.createConnection = function() {
const self = this;
Copy link
Member

Choose a reason for hiding this comment

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

how about eliminating the need for self by converting the only consuming function to an arrow function?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did that in a couple of other places, seems like I missed it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh I remember why I did not do it, because it is simpler to remove the listener this way. Can still refactor it, but it's not pretty.

const socket = new net.Socket();
class Agent extends http.Agent {
createConnection() {
const self = this;
const socket = new net.Socket();

socket.on('error', function() {
socket.push('HTTP/1.1 200\r\n\r\n');
});
socket.on('error', function() {
socket.push('HTTP/1.1 200\r\n\r\n');
});

socket.on('newListener', function onNewListener(name) {
if (name !== 'error')
return;
socket.removeListener('newListener', onNewListener);
socket.on('newListener', function onNewListener(name) {
if (name !== 'error')
return;
socket.removeListener('newListener', onNewListener);

// Let other listeners to be set up too
process.nextTick(function() {
self.breakSocket(socket);
// Let other listeners to be set up too
process.nextTick(function() {
self.breakSocket(socket);
});
});
});

return socket;
};
return socket;
}

Agent.prototype.breakSocket = function breakSocket(socket) {
socket.emit('error', new Error('Intentional error'));
};
breakSocket(socket) {
socket.emit('error', new Error('Intentional error'));
}
}

const agent = new Agent();

Expand Down
50 changes: 23 additions & 27 deletions test/parallel/test-http-client-readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,41 +23,37 @@
const common = require('../common');
const assert = require('assert');
const http = require('http');
const util = require('util');

const Duplex = require('stream').Duplex;

function FakeAgent() {
http.Agent.call(this);
}
util.inherits(FakeAgent, http.Agent);

FakeAgent.prototype.createConnection = function() {
const s = new Duplex();
let once = false;
class FakeAgent extends http.Agent {
createConnection() {
const s = new Duplex();
let once = false;

s._read = function() {
if (once)
return this.push(null);
once = true;
s._read = function() {
if (once)
return this.push(null);
once = true;

this.push('HTTP/1.1 200 Ok\r\nTransfer-Encoding: chunked\r\n\r\n');
this.push('b\r\nhello world\r\n');
this.readable = false;
this.push('0\r\n\r\n');
};
this.push('HTTP/1.1 200 Ok\r\nTransfer-Encoding: chunked\r\n\r\n');
this.push('b\r\nhello world\r\n');
this.readable = false;
this.push('0\r\n\r\n');
};

// Blackhole
s._write = function(data, enc, cb) {
cb();
};
// Blackhole
s._write = function(data, enc, cb) {
cb();
};

s.destroy = s.destroySoon = function() {
this.writable = false;
};
s.destroy = s.destroySoon = function() {
this.writable = false;
};

return s;
};
return s;
}
}

let received = '';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@
const common = require('../common');
const http = require('http');
const assert = require('assert');
const util = require('util');
const stream = require('stream');

// Verify that when piping a stream to an `OutgoingMessage` (or a type that
// inherits from `OutgoingMessage`), if data is emitted after the
// `OutgoingMessage` was closed - a `write after end` error is raised

function MyStream() {
stream.call(this);
}
util.inherits(MyStream, stream);
class MyStream extends stream {}

const server = http.createServer(common.mustCall(function(req, res) {
const myStream = new MyStream();
Expand Down
13 changes: 5 additions & 8 deletions test/parallel/test-readline-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@ const assert = require('assert');
const readline = require('readline');
const internalReadline = require('internal/readline');
const EventEmitter = require('events').EventEmitter;
const inherits = require('util').inherits;
const { Writable, Readable } = require('stream');

function FakeInput() {
EventEmitter.call(this);
class FakeInput extends EventEmitter {
resume() {}
pause() {}
write() {}
end() {}
}
inherits(FakeInput, EventEmitter);
FakeInput.prototype.resume = () => {};
FakeInput.prototype.pause = () => {};
FakeInput.prototype.write = () => {};
FakeInput.prototype.end = () => {};

function isWarned(emitter) {
for (const name in emitter) {
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-readline-keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@
const common = require('../common');
const PassThrough = require('stream').PassThrough;
const assert = require('assert');
const inherits = require('util').inherits;
const Interface = require('readline').Interface;


function FakeInput() {
PassThrough.call(this);
}
inherits(FakeInput, PassThrough);
class FakeInput extends PassThrough {}

function extend(k) {
return Object.assign({ ctrl: false, meta: false, shift: false }, k);
Expand Down
31 changes: 13 additions & 18 deletions test/parallel/test-stream-big-packet.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,26 @@
'use strict';
require('../common');
const assert = require('assert');
const util = require('util');
const stream = require('stream');

let passed = false;

function PassThrough() {
stream.Transform.call(this);
class PassThrough extends stream.Transform {
_transform(chunk, encoding, done) {
this.push(chunk);
done();
}
}
util.inherits(PassThrough, stream.Transform);
PassThrough.prototype._transform = function(chunk, encoding, done) {
this.push(chunk);
done();
};

function TestStream() {
stream.Transform.call(this);
}
util.inherits(TestStream, stream.Transform);
TestStream.prototype._transform = function(chunk, encoding, done) {
if (!passed) {
// Char 'a' only exists in the last write
passed = chunk.toString().includes('a');
class TestStream extends stream.Transform {
_transform(chunk, encoding, done) {
if (!passed) {
// Char 'a' only exists in the last write
passed = chunk.toString().includes('a');
}
done();
}
done();
};
}

const s1 = new PassThrough();
const s2 = new PassThrough();
Expand Down
29 changes: 13 additions & 16 deletions test/parallel/test-stream-events-prepend.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,23 @@
'use strict';
const common = require('../common');
const stream = require('stream');
const util = require('util');

function Writable() {
this.writable = true;
stream.Writable.call(this);
this.prependListener = undefined;
class Writable extends stream.Writable {
constructor() {
super();
this.prependListener = undefined;
}

_write(chunk, end, cb) {
cb();
}
}
util.inherits(Writable, stream.Writable);
Writable.prototype._write = function(chunk, end, cb) {
cb();
};

function Readable() {
this.readable = true;
stream.Readable.call(this);
class Readable extends stream.Readable {
_read() {
this.push(null);
}
}
util.inherits(Readable, stream.Readable);
Readable.prototype._read = function() {
this.push(null);
};

const w = new Writable();
w.on('pipe', common.mustCall());
Expand Down
Loading