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.

Sign up for GitHub

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

Jump to bottom

set autostart to false #36

Closed
wants to merge 6 commits into from
Closed

set autostart to false #36

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10e5b51
chore set autostart to false
jeswr Feb 14, 2022
f216e07
chore: fix setImmediate error
jeswr Feb 14, 2022
bdc53ed
fix: correctly apply autoStart on unionIterator
jeswr Feb 15, 2022
8bf9492
chore: fix test descriptions and remove unecessary autostarts
jeswr Feb 15, 2022
e21fa90
fix: fix setImmediate test
jeswr Feb 15, 2022
6f3af33
Simplify autoStart assignment.
RubenVerborgh Feb 15, 2022
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
24 changes: 18 additions & 6 deletions asynciterator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ export class AsyncIterator<T> extends EventEmitter {
After this operation, only read the returned iterator instead of the current one.
@param {object|Function} [options] Settings of the iterator, or the transformation function
@param {integer} [options.maxbufferSize=4] The maximum number of items to keep in the buffer
@param {boolean} [options.autoStart=true] Whether buffering starts directly after construction
@param {boolean} [options.autoStart=false] Whether buffering starts directly after construction
@param {integer} [options.offset] The number of items to skip
@param {integer} [options.limit] The maximum number of items
@param {Function} [options.filter] A function to synchronously filter items from the source
Expand Down Expand Up @@ -606,7 +606,7 @@ export class ArrayIterator<T> extends AsyncIterator<T> {
Creates a new `ArrayIterator`.
@param {Array} items The items that will be emitted.
*/
constructor(items?: Iterable<T>, { autoStart = true } = {}) {
constructor(items?: Iterable<T>, { autoStart = false } = {}) {
super();
const buffer = items ? [...items] : [];
this._sourceStarted = autoStart !== false;
Expand Down Expand Up @@ -729,9 +729,9 @@ export class BufferedIterator<T> extends AsyncIterator<T> {
Creates a new `BufferedIterator`.
@param {object} [options] Settings of the iterator
@param {integer} [options.maxBufferSize=4] The number of items to preload in the internal buffer
@param {boolean} [options.autoStart=true] Whether buffering starts directly after construction
@param {boolean} [options.autoStart=false] Whether buffering starts directly after construction
*/
constructor({ maxBufferSize = 4, autoStart = true } = {}) {
constructor({ maxBufferSize = 4, autoStart = false } = {}) {
super(INIT);
this.maxBufferSize = maxBufferSize;
taskScheduler(() => this._init(autoStart));
Expand Down Expand Up @@ -1220,6 +1220,18 @@ export class SimpleTransformIterator<S, D = S> extends TransformIterator<S, D> {
TransformOptions<S, D> & ((item: S, done: () => void) => void)) {
super(source, options as TransformIteratorOptions<S>);

if (typeof options === 'object') {
RubenVerborgh marked this conversation as resolved.
Show resolved Hide resolved
let removeOptions = true;
for (const key in options) {
if (key !== 'autoStart') {
removeOptions = false;
break;
}
}
if (removeOptions)
options = undefined;
}

// Set transformation steps from the options
options = options || (!isSourceExpression(source) ? source : null as any);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the only cause of some problems in the testing - since passing option = { autoStart: true } means that the source is not used even if it is not a source expression.

I've got around this by setting the options to undefined if the only key is autoStart - but I'm thinking that the tests should just be changed instead

Copy link
Owner

Choose a reason for hiding this comment

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

I'm sorry, I've been looking at this for 15 minutes now but still don't understand.

So the logic of the SimpleTransformIterator is that both of its parameters (source and options) are optional.

So what the original line 1224 is doing is determining whether options is the first or the second argument (given that source is optional, it might change position). The easy case is when options is non-nullish: then options is the second argument. If options is nullish, then options is the first argument—if that first argument is not a source.

None of that should change, regardless of the autoStart default. So probably a test change indeed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense - will do a test change then :)

if (options) {
Expand Down Expand Up @@ -1346,7 +1358,7 @@ export class MultiTransformIterator<S, D = S> extends TransformIterator<S, D> {
@param {module:asynciterator.AsyncIterator|Readable} [source] The source this iterator generates items from
@param {object|Function} [options] Settings of the iterator, or the transformation function
@param {integer} [options.maxbufferSize=4] The maximum number of items to keep in the buffer
@param {boolean} [options.autoStart=true] Whether buffering starts directly after construction
@param {boolean} [options.autoStart=false] Whether buffering starts directly after construction
@param {module:asynciterator.AsyncIterator} [options.source] The source this iterator generates items from
@param {integer} [options.offset] The number of items to skip
@param {integer} [options.limit] The maximum number of items
Expand Down Expand Up @@ -1571,7 +1583,7 @@ export class ClonedIterator<T> extends TransformIterator<T> {
@param {module:asynciterator.AsyncIterator|Readable} [source] The source this iterator copies items from
*/
constructor(source: AsyncIterator<T>) {
super(source, { autoStart: false });
super(source);
this._reading = false;
}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"test:immediate": "npm run mocha -- --require test/config/useSetImmediate.js",
"mocha": "c8 mocha",
"lint": "eslint asynciterator.ts test",
"lint:fix": "eslint asynciterator.ts test --fix",
"docs": "npm run build:module && npm run jsdoc",
"jsdoc": "jsdoc -c jsdoc.json"
},
Expand Down
10 changes: 5 additions & 5 deletions test/ArrayIterator-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ describe('ArrayIterator', () => {
});
});

describe('An ArrayIterator without arguments', () => {
describe('An ArrayIterator undefined iterator and autostart', () => {
let iterator;
before(() => {
iterator = new ArrayIterator();
iterator = new ArrayIterator(undefined, { autoStart: true });
Copy link
Owner

Choose a reason for hiding this comment

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

That is a bit weird to me; I'll investigate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was using undefined to emulate the existing test with no sources. An empty array should work as well instead.

captureEvents(iterator, 'readable', 'end');
});

Expand Down Expand Up @@ -83,10 +83,10 @@ describe('ArrayIterator', () => {
});
});

describe('An ArrayIterator with an empty array', () => {
describe('An ArrayIterator with an empty array and autoStart', () => {
let iterator;
before(() => {
iterator = new ArrayIterator([]);
iterator = new ArrayIterator([], { autoStart: true });
captureEvents(iterator, 'readable', 'end');
});

Expand Down Expand Up @@ -130,7 +130,7 @@ describe('ArrayIterator', () => {
describe('An ArrayIterator with an empty array without autoStart', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
describe('An ArrayIterator with an empty array without autoStart', () => {
describe('An ArrayIterator with an empty array', () => {

let iterator;
before(() => {
iterator = new ArrayIterator([], { autoStart: false });
iterator = new ArrayIterator([]);
captureEvents(iterator, 'readable', 'end');
});

Expand Down
60 changes: 30 additions & 30 deletions test/BufferedIterator-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator without arguments', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
Copy link
Owner

Choose a reason for hiding this comment

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

This is where this change will get a bit annoying, but I think it's important that the test structure remains the same. So first "without arguments" (so don't pass anything) and then the other cases.

(Note that the description and code are out of sync too.)

captureEvents(iterator, 'readable', 'end');
});

Expand Down Expand Up @@ -115,7 +115,7 @@ describe('BufferedIterator', () => {

describe('without autoStart', () => {
let iterator;
before(() => { iterator = createIterator({ autoStart: false }); });
before(() => { iterator = createIterator(); });

describe('before `read` has been called', () => {
it('should have emitted the `readable` event', () => {
Expand Down Expand Up @@ -225,7 +225,7 @@ describe('BufferedIterator', () => {

describe('with autoStart', () => {
let iterator;
before(() => { iterator = createIterator(); });
before(() => { iterator = createIterator({ autoStart: true }); });

describe('before `read` has been called', () => {
it('should not have emitted the `readable` event', () => {
Expand Down Expand Up @@ -312,7 +312,7 @@ describe('BufferedIterator', () => {

describe('without autoStart', () => {
let iterator;
before(() => { iterator = createIterator({ autoStart: false }); });
before(() => { iterator = createIterator(); });

describe('before `read` has been called', () => {
it('should have emitted the `readable` event', () => {
Expand Down Expand Up @@ -422,7 +422,7 @@ describe('BufferedIterator', () => {

describe('with autoStart', () => {
let iterator;
before(() => { iterator = createIterator(); });
before(() => { iterator = createIterator({ autoStart: true }); });

describe('before `read` has been called', () => {
it('should not have emitted the `readable` event', () => {
Expand Down Expand Up @@ -497,7 +497,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that is being closed', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
captureEvents(iterator, 'readable', 'end');
});

Expand Down Expand Up @@ -563,7 +563,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that is being closed while reading is in progress', () => {
let iterator, _readDone;
function createIterator() {
iterator = new BufferedIterator({ autoStart: false, maxBufferSize: 1 });
iterator = new BufferedIterator({ maxBufferSize: 1 });
iterator._read = (count, done) => { _readDone = done; };
sinon.spy(iterator, '_read');
captureEvents(iterator, 'readable', 'end');
Expand Down Expand Up @@ -1059,7 +1059,7 @@ describe('BufferedIterator', () => {

describe('without autoStart', () => {
let iterator;
before(() => { iterator = createIterator({ autoStart: false }); });
before(() => { iterator = createIterator(); });

it('should provide a readable `toString` representation', () => {
iterator.toString().should.equal('[BufferedIterator {buffer: 0}]');
Expand Down Expand Up @@ -1211,7 +1211,7 @@ describe('BufferedIterator', () => {

describe('with autoStart', () => {
let iterator;
before(() => { iterator = createIterator(); });
before(() => { iterator = createIterator({ autoStart: true }); });

it('should provide a readable `toString` representation', () => {
iterator.toString().should.equal('[BufferedIterator {next: a, buffer: 1}]');
Expand Down Expand Up @@ -1304,7 +1304,7 @@ describe('BufferedIterator', () => {

describe('with autoStart enabled', () => {
let iterator;
before(() => { iterator = createIterator(); });
before(() => { iterator = createIterator({ autoStart: true }); });

it('should provide a readable `toString` representation', () => {
iterator.toString().should.equal('[BufferedIterator {next: a, buffer: 3}]');
Expand Down Expand Up @@ -1406,7 +1406,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_read` that calls `done` multiple times', () => {
let iterator, readDone;
before(done => {
iterator = new BufferedIterator({ autoStart: false });
iterator = new BufferedIterator();
iterator._read = (count, callback) => { readDone = callback; };
scheduleTask(() => { iterator.read(); done(); });
});
Expand All @@ -1421,7 +1421,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_read` that does not call `done`', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._read = function () { this._push('a'); };
});

Expand All @@ -1438,7 +1438,7 @@ describe('BufferedIterator', () => {
let iterator;
before(() => {
let counter = 0;
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._read = function () { this.read(); this._push(counter++); };
});

Expand All @@ -1454,7 +1454,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with a synchronous beginning', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._begin = function (done) {
this._push('x');
this._push('y');
Expand Down Expand Up @@ -1575,7 +1575,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that pushes less than `maxBufferSize` items before _read', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
for (let i = 0; i < 3; i++)
iterator._push(i);
sinon.spy(iterator, '_read');
Expand All @@ -1590,7 +1590,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that pushes `maxBufferSize` items before _read', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
for (let i = 0; i < 4; i++)
iterator._push(i);
sinon.spy(iterator, '_read');
Expand All @@ -1604,7 +1604,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that starts reading before _read is called', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
// Forcibly change the status to 'reading',
// to test if the iterator deals with such an exceptional situation
iterator._changeState = function () { this._reading = true; };
Expand All @@ -1619,7 +1619,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator that closes before _completeClose is called', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator.close();
iterator._changeState(CLOSED);
sinon.spy(iterator, '_flush');
Expand All @@ -1633,7 +1633,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with an asynchronous beginning', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._begin = function (done) {
scheduleTask(() => {
this._push('x');
Expand Down Expand Up @@ -1756,7 +1756,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_begin` that calls `done` multiple times', () => {
let iterator, beginDone;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._begin = done => { beginDone = done; };
});

Expand All @@ -1770,7 +1770,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_begin` that does not call `done`', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._begin = function () { this._push('a'); };
iterator.close();
sinon.spy(iterator, '_read');
Expand Down Expand Up @@ -1817,7 +1817,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with a synchronous flush', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._read = function (item, done) {
this._push('a');
this.close();
Expand Down Expand Up @@ -1938,7 +1938,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with an asynchronous flush', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._read = function (item, done) {
this._push('a');
this.close();
Expand Down Expand Up @@ -2061,7 +2061,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_flush` that calls `done` multiple times', () => {
let iterator, flushDone;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._flush = done => { flushDone = done; };
iterator.close();
iterator.read();
Expand All @@ -2077,7 +2077,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with `_flush` that does not call `done`', () => {
let iterator;
before(() => {
iterator = new BufferedIterator();
iterator = new BufferedIterator({ autoStart: true });
iterator._flush = function () { this._push('a'); };
iterator.close();
captureEvents(iterator, 'end');
Expand Down Expand Up @@ -2111,7 +2111,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with a synchronous flush that is destroyed', () => {
let iterator;
before(() => {
iterator = new BufferedIterator({ maxBufferSize: 1 });
iterator = new BufferedIterator({ maxBufferSize: 1, autoStart: true });
iterator._read = function (item, done) {
this._push('a');
done();
Expand Down Expand Up @@ -2198,7 +2198,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator with an asynchronous flush that is destroyed', () => {
let iterator;
before(() => {
iterator = new BufferedIterator({ maxBufferSize: 1 });
iterator = new BufferedIterator({ maxBufferSize: 1, autoStart: true });
iterator._read = function (item, done) {
this._push('a');
done();
Expand Down Expand Up @@ -2312,7 +2312,7 @@ describe('BufferedIterator', () => {
describe('when changing the buffer size', () => {
let iterator;
before(() => {
iterator = new BufferedIterator({ maxBufferSize: 6 });
iterator = new BufferedIterator({ maxBufferSize: 6, autoStart: true });
iterator._read = sinon.spy(function (count, done) {
for (let i = 0; i < 4; i++)
this._push(i);
Expand Down Expand Up @@ -2359,7 +2359,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator created with an infinite maximum buffer size', () => {
let iterator, i = 0;
before(done => {
iterator = new BufferedIterator({ maxBufferSize: Infinity });
iterator = new BufferedIterator({ maxBufferSize: Infinity, autoStart: true });
iterator._read = sinon.spy(function (count, next) {
this._push(++i);
if (i === 10) {
Expand All @@ -2382,7 +2382,7 @@ describe('BufferedIterator', () => {
describe('A BufferedIterator create with a finite maximum buffer size', () => {
let iterator, i = 0, beforeDone;
before(() => {
iterator = new BufferedIterator({ maxBufferSize: 4 });
iterator = new BufferedIterator({ maxBufferSize: 4, autoStart: true });
iterator._read = sinon.spy(function (count, next) {
this._push(++i);
if (i === 10) {
Expand Down
4 changes: 2 additions & 2 deletions test/ClonedIterator-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('ClonedIterator', () => {

describe('Cloning an iterator that already has a destination', () => {
it('should throw an exception', () => {
const source = new AsyncIterator(), destination = new TransformIterator(source);
const source = new AsyncIterator(), destination = new TransformIterator(source, { autoStart: true });
source.should.have.property('_destination', destination);
(() => source.clone()).should.throw('The source already has a destination');
});
Expand Down Expand Up @@ -156,7 +156,7 @@ describe('ClonedIterator', () => {
});

describe('Cloning an iterator that asynchronously closes', () => {
function createIterator() { return new BufferedIterator(); }
function createIterator() { return new BufferedIterator({ autoStart: true }); }

function beforeClosing(getClone, getIterator, index) {
describe('before closing', () => {
Expand Down
Loading