diff --git a/package.json b/package.json index 3543aff92..0c8b4609d 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "exception-reporting": "0.41.4", "find-and-replace": "0.209.5", "fuzzy-finder": "1.5.8", - "github": "0.4.2", + "github": "0.5.0", "git-diff": "1.3.6", "go-to-line": "0.32.1", "grammar-selector": "0.49.5", diff --git a/spec/path-watcher-spec.js b/spec/path-watcher-spec.js index a805886e3..ae5cd7d75 100644 --- a/spec/path-watcher-spec.js +++ b/spec/path-watcher-spec.js @@ -48,21 +48,18 @@ describe('watchPath', function () { } describe('watchPath()', function () { - it('resolves getStartPromise() when the watcher begins listening', async function () { + it('resolves the returned promise when the watcher begins listening', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher = watchPath(rootDir, {}, () => {}) - await watcher.getStartPromise() + const watcher = await watchPath(rootDir, {}, () => {}) + expect(watcher.constructor.name).toBe('PathWatcher') }) it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = watchPath(rootDir, {}, () => {}) - await watcher0.getStartPromise() - - const watcher1 = watchPath(rootDir, {}, () => {}) - await watcher1.getStartPromise() + const watcher0 = await watchPath(rootDir, {}, () => {}) + const watcher1 = await watchPath(rootDir, {}, () => {}) expect(watcher0.native).toBe(watcher1.native) }) @@ -70,28 +67,21 @@ describe('watchPath', function () { it("reuses existing native watchers even while they're still starting", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = watchPath(rootDir, {}, () => {}) - await watcher0.getAttachedPromise() - expect(watcher0.native.isRunning()).toBe(false) - - const watcher1 = watchPath(rootDir, {}, () => {}) - await watcher1.getAttachedPromise() - + const [watcher0, watcher1] = await Promise.all([ + watchPath(rootDir, {}, () => {}), + watchPath(rootDir, {}, () => {}) + ]) expect(watcher0.native).toBe(watcher1.native) - - await Promise.all([watcher0.getStartPromise(), watcher1.getStartPromise()]) }) it("doesn't attach new watchers to a native watcher that's stopping", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = watchPath(rootDir, {}, () => {}) - await watcher0.getStartPromise() + const watcher0 = await watchPath(rootDir, {}, () => {}) const native0 = watcher0.native watcher0.dispose() - - const watcher1 = watchPath(rootDir, {}, () => {}) + const watcher1 = await watchPath(rootDir, {}, () => {}) expect(watcher1.native).not.toBe(native0) }) @@ -105,13 +95,8 @@ describe('watchPath', function () { await fs.mkdir(subDir) // Keep the watchers alive with an undisposed subscription - const rootWatcher = watchPath(rootDir, {}, () => {}) - const childWatcher = watchPath(subDir, {}, () => {}) - - await Promise.all([ - rootWatcher.getStartPromise(), - childWatcher.getStartPromise() - ]) + const rootWatcher = await watchPath(rootDir, {}, () => {}) + const childWatcher = await watchPath(subDir, {}, () => {}) expect(rootWatcher.native).toBe(childWatcher.native) expect(rootWatcher.native.isRunning()).toBe(true) @@ -120,13 +105,11 @@ describe('watchPath', function () { waitForChanges(rootWatcher, subFile), waitForChanges(childWatcher, subFile) ]) - await fs.writeFile(subFile, 'subfile\n', {encoding: 'utf8'}) await firstChanges const nextRootEvent = waitForChanges(rootWatcher, rootFile) await fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}) - await nextRootEvent }) @@ -149,23 +132,18 @@ describe('watchPath', function () { ]) // Begin the child watchers and keep them alive - const subWatcher0 = watchPath(subDir0, {}, () => {}) + const subWatcher0 = await watchPath(subDir0, {}, () => {}) const subWatcherChanges0 = waitForChanges(subWatcher0, subFile0) - const subWatcher1 = watchPath(subDir1, {}, () => {}) + const subWatcher1 = await watchPath(subDir1, {}, () => {}) const subWatcherChanges1 = waitForChanges(subWatcher1, subFile1) - await Promise.all( - [subWatcher0, subWatcher1].map(watcher => watcher.getStartPromise()) - ) expect(subWatcher0.native).not.toBe(subWatcher1.native) // Create the parent watcher - const parentWatcher = watchPath(parentDir, {}, () => {}) + const parentWatcher = await watchPath(parentDir, {}, () => {}) const parentWatcherChanges = waitForChanges(parentWatcher, rootFile, subFile0, subFile1) - await parentWatcher.getStartPromise() - expect(subWatcher0.native).toBe(parentWatcher.native) expect(subWatcher1.native).toBe(parentWatcher.native) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 059208cbd..4ce84617a 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -585,10 +585,10 @@ describe "Project", -> waitsForPromise -> stopAllWatchers() runs -> atom.project.setPaths([dirOne]) - waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() + waitsForPromise -> atom.project.getWatcherPromise dirOne runs -> - expect(atom.project.watchersByPath[dirTwo]).toEqual undefined + expect(atom.project.watcherPromisesByPath[dirTwo]).toEqual undefined fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index a779f78f5..798ff0619 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -431,6 +431,14 @@ class NativeWatcherRegistry { watcher.attachToNative(native, nativePath) }) } + + // Private: Generate a visual representation of the currently active watchers managed by this + // registry. + // + // Returns a {String} showing the tree structure. + print () { + return this.tree.print() + } } module.exports = {NativeWatcherRegistry} diff --git a/src/path-watcher.js b/src/path-watcher.js index 001b17818..69bf36da0 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -339,7 +339,7 @@ class NativeWatcher { // ```js // const {watchPath} = require('atom') // -// const disposable = watchPath('/var/log', {}, events => { +// const disposable = await watchPath('/var/log', {}, events => { // console.log(`Received batch of ${events.length} events.`) // for (const event of events) { // // "created", "modified", "deleted", "renamed" @@ -424,6 +424,8 @@ class PathWatcher { // intend to assert about because there will be a delay between the instantiation of the watcher and the activation // of the underlying OS resources that feed it events. // + // PathWatchers acquired through `watchPath` are already started. + // // ```js // const {watchPath} = require('atom') // const ROOT = path.join(__dirname, 'fixtures') @@ -505,14 +507,16 @@ class PathWatcher { })) this.subs.add(native.onShouldDetach(({replacement, watchedPath}) => { - if (replacement !== native && this.normalizedPath.startsWith(watchedPath)) { + if (this.native === native && replacement !== native && this.normalizedPath.startsWith(watchedPath)) { this.attachToNative(replacement) } })) this.subs.add(native.onWillStop(() => { - this.subs.dispose() - this.native = null + if (this.native === native) { + this.subs.dispose() + this.native = null + } })) this.resolveAttachedPromise() @@ -579,6 +583,11 @@ class PathWatcherManager { return watcher } + // Private: Return a {String} depicting the currently active native watchers. + print () { + return this.nativeRegistry.print() + } + // Private: Stop all living watchers. // // Returns a {Promise} that resolves when all native watcher resources are disposed. @@ -604,13 +613,13 @@ class PathWatcherManager { // * `path` {String} containing the absolute path to the filesystem entry that was acted upon. // * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. // -// Returns a {PathWatcher}. Note that every {PathWatcher} is a {Disposable}, so they can be managed by -// [CompositeDisposables]{CompositeDisposable} if desired. +// Returns a {Promise} that will resolve to a {PathWatcher} once it has started. Note that every {PathWatcher} +// is a {Disposable}, so they can be managed by a {CompositeDisposable} if desired. // // ```js // const {watchPath} = require('atom') // -// const disposable = watchPath('/var/log', {}, events => { +// const disposable = await watchPath('/var/log', {}, events => { // console.log(`Received batch of ${events.length} events.`) // for (const event of events) { // // "created", "modified", "deleted", "renamed" @@ -629,7 +638,8 @@ class PathWatcherManager { // ``` // function watchPath (rootPath, options, eventCallback) { - return PathWatcherManager.instance().createWatcher(rootPath, options, eventCallback) + const watcher = PathWatcherManager.instance().createWatcher(rootPath, options, eventCallback) + return watcher.getStartPromise().then(() => watcher) } // Private: Return a Promise that resolves when all {NativeWatcher} instances associated with a FileSystemManager @@ -638,4 +648,9 @@ function stopAllWatchers () { return PathWatcherManager.instance().stopAllWatchers() } -module.exports = {watchPath, stopAllWatchers} +// Private: Show the currently active native watchers. +function printWatchers () { + return PathWatcherManager.instance().print() +} + +module.exports = {watchPath, stopAllWatchers, printWatchers} diff --git a/src/project.coffee b/src/project.coffee index 4564e37bb..756fd756a 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -29,13 +29,13 @@ class Project extends Model @repositoryPromisesByPath = new Map() @repositoryProviders = [new GitRepositoryProvider(this, config)] @loadPromisesByPath = {} - @watchersByPath = {} + @watcherPromisesByPath = {} @consumeServices(packageManager) destroyed: -> buffer.destroy() for buffer in @buffers.slice() repository?.destroy() for repository in @repositories.slice() - watcher.dispose() for _, watcher in @watchersByPath + watcher.dispose() for _, watcher in @watcherPromisesByPath @rootDirectories = [] @repositories = [] @@ -140,6 +140,10 @@ class Project extends Model # # To watch paths outside of open projects, use the `watchPaths` function instead; see {PathWatcher}. # + # When writing tests against functionality that uses this method, be sure to wait for the + # {Promise} returned by {getWatcherPromise()} before manipulating the filesystem to ensure that + # the watcher is receiving events. + # # * `callback` {Function} to be called with batches of filesystem events reported by # the operating system. # * `events` An {Array} of objects that describe a batch of filesystem events. @@ -212,8 +216,8 @@ class Project extends Model @rootDirectories = [] @repositories = [] - watcher.dispose() for _, watcher in @watchersByPath - @watchersByPath = {} + watcher.then((w) -> w.dispose()) for _, watcher in @watcherPromisesByPath + @watcherPromisesByPath = {} @addPath(projectPath, emitEvent: false) for projectPath in projectPaths @@ -229,11 +233,15 @@ class Project extends Model return if existingDirectory.getPath() is directory.getPath() @rootDirectories.push(directory) - @watchersByPath[directory.getPath()] = watchPath directory.getPath(), {}, (events) => - @emitter.emit 'did-change-files', events + @watcherPromisesByPath[directory.getPath()] = watchPath directory.getPath(), {}, (events) => + # Stop event delivery immediately on removal of a rootDirectory, even if its watcher + # promise has yet to resolve at the time of removal + if @rootDirectories.includes directory + @emitter.emit 'did-change-files', events - for root, watcher in @watchersByPath - watcher.dispose() unless @rootDirectoryies.includes root + for root, watcherPromise in @watcherPromisesByPath + unless @rootDirectories.includes root + watcherPromise.then (watcher) -> watcher.dispose() repo = null for provider in @repositoryProviders @@ -250,6 +258,21 @@ class Project extends Model directory ?= @defaultDirectoryProvider.directoryForURISync(projectPath) directory + # Extended: Access a {Promise} that resolves when the filesystem watcher associated with a project + # root directory is ready to begin receiving events. + # + # This is especially useful in test cases, where it's important to know that the watcher is + # ready before manipulating the filesystem to produce events. + # + # * `projectPath` {String} One of the project's root directories. + # + # Returns a {Promise} that resolves with the {PathWatcher} associated with this project root + # once it has initialized and is ready to start sending events. The Promise will reject with + # an error instead if `projectPath` is not currently a root directory. + getWatcherPromise: (projectPath) -> + @watcherPromisesByPath[projectPath] or + Promise.reject(new Error("#{projectPath} is not a project root")) + # Public: remove a path from the project's list of root paths. # # * `projectPath` {String} The path to remove. @@ -268,7 +291,8 @@ class Project extends Model [removedDirectory] = @rootDirectories.splice(indexToRemove, 1) [removedRepository] = @repositories.splice(indexToRemove, 1) removedRepository?.destroy() unless removedRepository in @repositories - @watchersByPath[projectPath]?.dispose() + @watcherPromisesByPath[projectPath]?.then (w) -> w.dispose() + delete @watcherPromisesByPath[projectPath] @emitter.emit "did-change-paths", @getPaths() true else