diff --git a/.jshintrc b/.jshintrc index 17b577845..9f0a2639b 100644 --- a/.jshintrc +++ b/.jshintrc @@ -18,7 +18,8 @@ "maxlen": 80, "node": true, "predef": [ - "Promise" + "Promise", + "Set" ], "proto": true, "quotmark": "double", diff --git a/lib/tree.js b/lib/tree.js index 138099c2e..c85a925bc 100644 --- a/lib/tree.js +++ b/lib/tree.js @@ -127,6 +127,8 @@ Tree.prototype.walk = function(blobsOnly) { var event = new events.EventEmitter(); var total = 1; + var entries = new Set(); + var finalEntires = []; // This looks like a DFS, but it is a BFS because of implicit queueing in // the recursive call to `entry.getTree(bfs)` @@ -136,11 +138,15 @@ Tree.prototype.walk = function(blobsOnly) { if (error) { return event.emit("error", error); } - var entries = tree.entries(); - entries.forEach(function (entry, entryIndex) { - if (!blobsOnly || entry.isFile()) { + + tree.entries().forEach(function (entry, entryIndex) { + if (!blobsOnly || entry.isFile() && !entries.has(entry)) { event.emit("entry", entry); - entries.push(entry); + entries.add(entry); + + // Node 0.12 doesn't support either [v for (v of entries)] nor + // Array.from so we'll just maintain our own list. + finalEntires.push(entry); } if (entry.isTree()) { @@ -150,7 +156,7 @@ Tree.prototype.walk = function(blobsOnly) { }); if (total === 0) { - event.emit("end", entries); + event.emit("end", finalEntires); } } diff --git a/test/tests/tree.js b/test/tests/tree.js new file mode 100644 index 000000000..630ba9cca --- /dev/null +++ b/test/tests/tree.js @@ -0,0 +1,81 @@ +var assert = require("assert"); +var path = require("path"); +var local = path.join.bind(path, __dirname); +var promisify = require("promisify-node"); +var fse = promisify(require("fs-extra")); + +describe("Tree", function() { + var RepoUtils = require("../utils/repository_setup"); + + var repoPath = local("../repos/tree"); + + beforeEach(function() { + var test = this; + return RepoUtils.createRepository(repoPath) + .then(function(repo) { + test.repository = repo; + }); + }); + + after(function() { + return fse.remove(repoPath); + }); + + it("walks its entries and returns the same entries on both progress and end", + function() { + var repo = this.repository; + var file1 = "test.txt"; + var file2 = "foo/bar.txt"; + // index.addByPath doesn't like \s so normalize only for the expected paths + var expectedPaths = [file1, path.normalize(file2)]; + var progressEntries = []; + var endEntries; + + return RepoUtils.commitFileToRepo(repo, file1, "") + .then(function(commit) { + return RepoUtils.commitFileToRepo(repo, file2, "", commit); + }) + .then(function(commit) { + return commit.getTree(); + }) + .then(function(tree) { + assert(tree); + + return new Promise(function (resolve, reject) { + var walker = tree.walk(); + + walker.on("entry", function(entry) { + progressEntries.push(entry); + }); + walker.on("end", function(entries) { + endEntries = entries; + resolve(); + }); + walker.on("error", reject); + + walker.start(); + }); + }) + .then(function() { + assert(progressEntries.length); + assert(endEntries && endEntries.length); + + function getEntryPath(entry) { + return entry.path(); + } + + var progressFilePaths = progressEntries.map(getEntryPath); + var endFilePaths = endEntries.map(getEntryPath); + + assert.deepEqual( + expectedPaths, progressFilePaths, + "progress entry paths do not match expected paths" + ); + + assert.deepEqual( + expectedPaths, endFilePaths, + "end entry paths do not match expected paths" + ); + }); + }); +}); diff --git a/test/utils/repository_setup.js b/test/utils/repository_setup.js index 26384bd91..d48c04650 100644 --- a/test/utils/repository_setup.js +++ b/test/utils/repository_setup.js @@ -18,18 +18,28 @@ var RepositorySetup = { }, commitFileToRepo: - function commitFileToRepo(repository, fileName, fileContent) { + function commitFileToRepo(repository, fileName, fileContent, parentCommit) { var repoWorkDir = repository.workdir(); var signature = NodeGit.Signature.create("Foo bar", "foo@bar.com", 123456789, 60); - return fse.writeFile(path.join(repoWorkDir, fileName), fileContent) + var filePath = path.join(repoWorkDir, fileName); + var parents = []; + if (parentCommit) { + parents.push(parentCommit); + } + + // fse.ensure allows us to write files inside new folders + return fse.ensureFile(filePath) + .then(function() { + return fse.writeFile(filePath, fileContent); + }) .then(function() { return RepositorySetup.addFileToIndex(repository, fileName); }) .then(function(oid) { return repository.createCommit("HEAD", signature, signature, - "initial commit", oid, []); + "initial commit", oid, parents); }) .then(function(commitOid) { return repository.getCommit(commitOid);