Skip to content

Commit

Permalink
[BUGFIX] Ensure build annotation is correctly passed to the builder, …
Browse files Browse the repository at this point in the history
…and fulfilled via watcher.currentBuild.

This feature is used heavily by ember-cli, but was previously insufficiently tested here. My small refactoring, converting code from promise chaining to async await, inadvertently broke this.
  • Loading branch information
stefanpenner committed Feb 29, 2020
1 parent 62dcba5 commit 67bca7a
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 32 deletions.
24 changes: 13 additions & 11 deletions lib/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Watcher extends EventEmitter {
return this._currentBuild;
}

// TODO: this is an interim solution, pending a largerly cleanup of this class.
// TODO: this is an interim solution, pending a largely cleanup of this class.
// Currently I can rely on understanding the various pieces of this class, to
// know this is safe. This is not a good long term solution, but given
// relatively little time to address this currently, it is "ok". I do plan,
Expand Down Expand Up @@ -90,17 +90,19 @@ class Watcher extends EventEmitter {

this.watcherAdapter.on('change', this._change.bind(this));
this.watcherAdapter.on('error', this._error.bind(this));
this._updateCurrentBuild((async () => {
try {
await this.watcherAdapter.watch();
logger.debug('ready');
this._ready = true;
} catch(e) {
this._error(e);
}
this._updateCurrentBuild(
(async () => {
try {
await this.watcherAdapter.watch();
logger.debug('ready');
this._ready = true;
} catch (e) {
this._error(e);
}

await this._build();
})());
return this._build();
})()
);

return this._lifetime.promise;
}
Expand Down
64 changes: 43 additions & 21 deletions test/watcher_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@ describe('Watcher', function() {
watched: true,
};

const buildResults = {};

const builder = {
async build() {
return buildResults;
async build(_, buildAnnotation) {
return buildAnnotation;
},
};

Expand All @@ -59,7 +57,14 @@ describe('Watcher', function() {
expect(adapterOn).to.have.been.calledWith('change');
expect(adapterOn).to.have.been.calledWith('error');

await watcher.currentBuild;
const result = await watcher.currentBuild;
expect(result).to.eql({
type: 'initial',
reason: 'watcher',
primaryFile: undefined,
changedFiles: [],
filePath: undefined,
});
expect(adapterWatch).to.have.been.called;
expect(trigger).to.have.been.calledWith('buildStart');
expect(trigger).to.have.been.calledWith('buildSuccess');
Expand Down Expand Up @@ -119,7 +124,14 @@ describe('Watcher', function() {
expect(changeHandler).to.have.been.calledWith('change', 'file.js', 'root');

const result = await watcher.currentBuild;
expect(result.filePath).to.equal(path.join('root', 'file.js'));

expect(result).to.eql({
type: 'rebuild',
reason: 'watcher',
primaryFile: path.join('root', 'file.js'),
changedFiles: [path.join('root', 'file.js')],
filePath: path.join('root', 'file.js'),
});

expect(debounceHandler).to.have.been.called;
expect(buildStartHandler).to.have.been.called;
Expand Down Expand Up @@ -171,27 +183,30 @@ describe('Watcher', function() {
const watcher = new Watcher(builder, [watchedNodeBasic], { watcherAdapter: adapter });

await watcher._build();

expect(builderBuild.args[0][1]).to.deep.equal({
type: 'initial',
reason: 'watcher',
primaryFile: undefined,
filePath: undefined,
changedFiles: [],
});
});

it('annotation is properly sent on rebuild', function() {
it('annotation is properly sent on rebuild', async function() {
const builderBuild = sinon.spy(builder, 'build');
const watcher = new Watcher(builder, [watchedNodeBasic], { watcherAdapter: adapter });

watcher._changedFiles = [path.join('root', 'file.js')];

return watcher._build(path.join('root', 'file.js')).then(() => {
expect(builderBuild.args[0][1]).to.deep.equal({
type: 'rebuild',
reason: 'watcher',
primaryFile: path.join('root', 'file.js'),
changedFiles: [path.join('root', 'file.js')],
});
await watcher._build(path.join('root', 'file.js'));

expect(builderBuild.args[0][1]).to.deep.equal({
type: 'rebuild',
reason: 'watcher',
primaryFile: path.join('root', 'file.js'),
filePath: path.join('root', 'file.js'),
changedFiles: [path.join('root', 'file.js')],
});
});
});
Expand Down Expand Up @@ -230,7 +245,7 @@ describe('Watcher', function() {
});

describe('quit', function() {
it('quits the watcher', function() {
it('quits the watcher', async function() {
const adapterQuit = sinon.spy(adapter, 'quit');
const watcher = new Watcher(builder, [watchedNodeBasic], { watcherAdapter: adapter });

Expand All @@ -241,13 +256,20 @@ describe('Watcher', function() {

watcher.start();

return watcher.currentBuild.then(() => {
return watcher.quit().then(() => {
expect(adapterQuit).to.have.been.called;
expect(quitStartHandler).to.have.been.called;
expect(quitEndHandler).to.have.been.called;
});
const result = await watcher.currentBuild;

expect(result).to.eql({
type: 'initial',
reason: 'watcher',
primaryFile: undefined,
changedFiles: [],
filePath: undefined,
});

await watcher.quit();
expect(adapterQuit).to.have.been.called;
expect(quitStartHandler).to.have.been.called;
expect(quitEndHandler).to.have.been.called;
});

it('does nothing if already quitting', function() {
Expand Down

0 comments on commit 67bca7a

Please sign in to comment.