From 8a6ab81325850c5eda5e555eae351f8dfa1c0d71 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 2 Dec 2015 13:26:06 -0500 Subject: [PATCH] 100% less racy. --- spec/git-repository-async-spec.js | 22 ++++++++-------------- src/git-repository-async.js | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/spec/git-repository-async-spec.js b/spec/git-repository-async-spec.js index 37d5cb898..829c33fcd 100644 --- a/spec/git-repository-async-spec.js +++ b/spec/git-repository-async-spec.js @@ -296,11 +296,9 @@ describe('GitRepositoryAsync-js', () => { // When the path is added to the project, the repository is refreshed. We // need to wait for that to complete before the tests continue so that - // we're in a known state. *But* it's really hard to observe that from the - // outside in a non-racy fashion. So let's refresh again and wait for it - // to complete before we continue. + // we're in a known state. repository = atom.project.getRepositories()[0].async - waitsForPromise(() => repository.refreshStatus()) + waitsFor(() => !repository._isRefreshing()) }) it('emits a status-changed event when a buffer is saved', async () => { @@ -329,14 +327,6 @@ describe('GitRepositoryAsync-js', () => { runs(() => { expect(statusHandler.callCount).toBe(1) expect(statusHandler).toHaveBeenCalledWith({path: editor.getPath(), pathStatus: 256}) - - const buffer = editor.getBuffer() - const reloadHandler = jasmine.createSpy('reloadHandler') - buffer.onDidReload(reloadHandler) - buffer.reload() - - waitsFor(() => reloadHandler.callCount > 0) - runs(() => expect(reloadHandler.callCount).toBe(1)) }) }) @@ -381,7 +371,7 @@ describe('GitRepositoryAsync-js', () => { // See the comment in the 'buffer events' beforeEach for why we need to do // this. const repository = atom.project.getRepositories()[0].async - waitsForPromise(() => repository.refreshStatus()) + waitsFor(() => !repository._isRefreshing()) }) afterEach(() => { @@ -393,6 +383,10 @@ describe('GitRepositoryAsync-js', () => { project2 = new Project({notificationManager: atom.notifications, packageManager: atom.packages, confirm: atom.confirm}) project2.deserialize(atom.project.serialize(), atom.deserializers) + + const repo = project2.getRepositories()[0].async + waitsFor(() => !repo._isRefreshing()) + const buffer = project2.getBuffers()[0] waitsFor(() => buffer.loaded) @@ -400,7 +394,7 @@ describe('GitRepositoryAsync-js', () => { buffer.append('changes') const statusHandler = jasmine.createSpy('statusHandler') - project2.getRepositories()[0].async.onDidChangeStatus(statusHandler) + repo.onDidChangeStatus(statusHandler) buffer.save() waitsFor(() => statusHandler.callCount > 0) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index 4f74cf620..3475684a1 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -30,6 +30,7 @@ module.exports = class GitRepositoryAsync { this.pathStatusCache = {} this.repoPromise = Git.Repository.open(path) this.isCaseInsensitive = fs.isCaseInsensitive() + this._refreshingCount = 0 const {project} = options this.project = project @@ -174,15 +175,21 @@ module.exports = class GitRepositoryAsync { .then(branchRef => this.branch = branchRef) } - // Refreshes the git status. Note: the sync GitRepository class does this with - // a separate process, let's see if we can avoid that. + // Refreshes the git status. + // + // Returns :: Promise + // Resolves when refresh has completed. refreshStatus () { + this._refreshingCount++ + // TODO add upstream, branch, and submodule tracking const status = this.repoPromise .then(repo => repo.getStatus()) .then(statuses => { + console.log(Object.keys(statuses)) // update the status cache - return Promise.all(statuses.map(status => [status.path(), status.statusBit()])) + const statusPairs = statuses.map(status => [status.path(), status.statusBit()]) + return Promise.all(statusPairs) .then(statusesByPath => _.object(statusesByPath)) }) .then(newPathStatusCache => { @@ -195,12 +202,16 @@ module.exports = class GitRepositoryAsync { const branch = this._refreshBranch() - return Promise.all([status, branch]) + return Promise.all([status, branch]).then(_ => this._refreshingCount--) } // Section: Private // ================ + _isRefreshing () { + return this._refreshingCount === 0 + } + subscribeToBuffer (buffer) { const bufferSubscriptions = new CompositeDisposable()