diff --git a/docs/config/01-configuration-file.md b/docs/config/01-configuration-file.md index a97730693..6deb8c1f6 100644 --- a/docs/config/01-configuration-file.md +++ b/docs/config/01-configuration-file.md @@ -89,8 +89,9 @@ These are all of the available configuration options. **Description:** When Karma is watching the files for changes, it tries to batch multiple changes into a single run so that the test runner doesn't try to start and restart running -tests more than it should. The configuration setting tells Karma how long to wait (in milliseconds) after any changes -have occurred before starting the test process again. +tests more than it should, or restart while build files are not in a consistent state. The configuration setting +tells Karma how long to wait (in milliseconds) from the last file change before starting +the test process again, resetting the timer each time a file changes (i.e. [debouncing](https://davidwalsh.name/javascript-debounce-function)). ## basePath diff --git a/lib/file-list.js b/lib/file-list.js index ca9c9e2e2..8aacf29a9 100644 --- a/lib/file-list.js +++ b/lib/file-list.js @@ -49,13 +49,13 @@ function byPath (a, b) { // emitter - EventEmitter // preprocess - Function // batchInterval - Number -var List = function (patterns, excludes, emitter, preprocess, batchInterval) { +var List = function (patterns, excludes, emitter, preprocess, autoWatchBatchDelay) { // Store options this._patterns = patterns this._excludes = excludes this._emitter = emitter this._preprocess = Promise.promisify(preprocess) - this._batchInterval = batchInterval + this._batchInterval = autoWatchBatchDelay // The actual list of files this.buckets = new Map() @@ -71,14 +71,14 @@ var List = function (patterns, excludes, emitter, preprocess, batchInterval) { var self = this // Emit the `file_list_modified` event. - // This function is throttled to the value of `batchInterval` + // This function is debounced to the value of `autoWatchBatchDelay` // to avoid spamming the listener. function emit () { self._emitter.emit('file_list_modified', self.files) } - var throttledEmit = _.throttle(emit, self._batchInterval, {leading: false}) + var debouncedEmit = _.debounce(emit, self._batchInterval) self._emitModified = function (immediate) { - immediate ? emit() : throttledEmit() + immediate ? emit() : debouncedEmit() } } diff --git a/test/unit/file-list.spec.js b/test/unit/file-list.spec.js index ccf10d897..2775a1b2f 100644 --- a/test/unit/file-list.spec.js +++ b/test/unit/file-list.spec.js @@ -442,7 +442,7 @@ describe('FileList', () => { 'graceful-fs': mockFs }) - list = new List(patterns('/some/*.js', '*.txt'), ['/secret/*.txt'], emitter, preprocess) + list = new List(patterns('/some/*.js', '*.txt'), ['/secret/*.txt'], emitter, preprocess, 1) }) it('does not add excluded files', () => { @@ -486,7 +486,7 @@ describe('FileList', () => { return list.refresh().then(() => { modified.reset() - return list.addFile('/some/d.js').then(() => { + return list.addFile('/some/d.js').delay(3).then(() => { expect(modified).to.have.been.calledOnce }) }) @@ -561,7 +561,7 @@ describe('FileList', () => { it('updates mtime and fires "file_list_modified"', () => { // MATCH: /some/a.js, /some/b.js - list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess) + list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess, 1) var modified = sinon.stub() emitter.on('file_list_modified', modified) @@ -569,7 +569,7 @@ describe('FileList', () => { mockFs._touchFile('/some/b.js', '2020-01-01') modified.reset() - return list.changeFile('/some/b.js').then((files) => { + return list.changeFile('/some/b.js').delay(3).then((files) => { expect(modified).to.have.been.calledOnce expect(findFile('/some/b.js', files.served).mtime).to.be.eql(new Date('2020-01-01')) }) @@ -654,7 +654,7 @@ describe('FileList', () => { it('removes the file from the list and fires "file_list_modified"', () => { // MATCH: /some/a.js, /some/b.js, /a.txt - list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess) + list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess, 1) var modified = sinon.stub() emitter.on('file_list_modified', modified) @@ -662,7 +662,7 @@ describe('FileList', () => { return list.refresh().then((files) => { modified.reset() return list.removeFile('/some/a.js') - }).then((files) => { + }).delay(3).then((files) => { expect(pathsFrom(files.served)).to.be.eql([ '/some/b.js', '/a.txt' @@ -685,6 +685,15 @@ describe('FileList', () => { }) describe('batch interval', () => { + // IMPORTANT: When writing tests for debouncing behaviour, you must wait for the promise + // returned by list.changeFile or list.addFile. list.removeFile calls self._emitModified() + // in a different manner and doesn't *need* to be waited on. If you use this behaviour + // in your tests it can can lead to very confusing results when they are modified or + // extended. + // + // Rule of thumb: Always wait on the promises returned by list.addFile, list.changeFile, + // and list.removeFile. + var clock = null beforeEach(() => { @@ -723,7 +732,97 @@ describe('FileList', () => { Promise.setScheduler((fn) => process.nextTick(fn)) }) - it('batches multiple changes within an interval', () => { + it('debounces calls to emitModified', () => { + list = new List(patterns(), [], emitter, preprocess, 100) + + return list.refresh().then(() => { + modified.reset() + list._emitModified() + clock.tick(99) + expect(modified).to.not.have.been.called + list._emitModified() + clock.tick(2) + expect(modified).to.not.have.been.called + clock.tick(97) + expect(modified).to.not.have.been.called + clock.tick(2) + expect(modified).to.have.been.calledOnce + clock.tick(1000) + expect(modified).to.have.been.calledOnce + list._emitModified() + clock.tick(99) + expect(modified).to.have.been.calledOnce + clock.tick(2) + expect(modified).to.have.been.calledTwice + }) + }) + + it('debounces a single file change', () => { + list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess, 100) + + return list.refresh().then((files) => { + modified.reset() + // Even with no changes, all these files are served + list.addFile('/some/0.js').then(() => { + clock.tick(99) + expect(modified).to.not.have.been.called + + clock.tick(2) + expect(modified).to.have.been.calledOnce + + files = modified.lastCall.args[0] + expect(pathsFrom(files.served)).to.be.eql([ + '/some/0.js', + '/some/a.js', + '/some/b.js', + '/a.txt' + ]) + }) + }) + }) + + it('debounces several changes to a file', () => { + list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess, 100) + + return list.refresh().then((files) => { + modified.reset() + list.addFile('/some/0.js').then(() => { + clock.tick(99) + expect(modified).to.not.have.been.called + + // Modify file, must change mtime too, or change is ignored + mockFs._touchFile('/some/0.js', '2020-01-01') + list.changeFile('/some/0.js').then(() => { + // Ensure that the debounce timer was reset + clock.tick(2) + expect(modified).to.not.have.been.called + + // Ensure that debounce timer fires after 100ms + clock.tick(99) + expect(modified).to.have.been.calledOnce + + // Make sure there aren't any lingering debounce calls + clock.tick(1000) + + // Modify file (one hour later mtime) + expect(modified).to.have.been.calledOnce + mockFs._touchFile('/some/0.js', '2020-01-02') + list.changeFile('/some/0.js').then(() => { + clock.tick(99) + expect(modified).to.have.been.calledOnce + clock.tick(2) + expect(modified).to.have.been.calledTwice + + // Make sure there aren't any lingering calls + clock.tick(1000) + expect(modified).to.have.been.calledTwice + }) + }) + }) + }) + }) + + it('debounces multiple changes until there is quiescence', () => { // MATCH: /some/a.js, /some/b.js, /a.txt list = new List(patterns('/some/*.js', '/a.*'), [], emitter, preprocess, 100) @@ -732,22 +831,29 @@ describe('FileList', () => { mockFs._touchFile('/some/b.js', '2020-01-01') list.changeFile('/some/b.js') list.removeFile('/some/a.js') // /some/b.js, /a.txt - list.removeFile('/a.txt') // /some/b.js list.addFile('/a.txt') // /some/b.js, /a.txt - list.addFile('/some/0.js') // /some/0.js, /some/b.js, /a.txt - - clock.tick(99) - expect(modified).to.not.have.been.called - - clock.tick(2) - expect(modified).to.have.been.calledOnce - - files = modified.lastCall.args[0] - expect(pathsFrom(files.served)).to.be.eql([ - '/some/0.js', - '/some/b.js', - '/a.txt' - ]) + list.addFile('/some/0.js').then(() => { // /some/0.js, /some/b.js, /a.txt + clock.tick(99) + expect(modified).to.not.have.been.called + mockFs._touchFile('/a.txt', '2020-01-01') + list.changeFile('/a.txt').then(() => { + clock.tick(2) + expect(modified).to.not.have.been.called + + clock.tick(100) + expect(modified).to.have.been.calledOnce + + clock.tick(1000) + expect(modified).to.have.been.calledOnce + + files = modified.lastCall.args[0] + expect(pathsFrom(files.served)).to.be.eql([ + '/some/0.js', + '/some/b.js', + '/a.txt' + ]) + }) + }) }) })