Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Traverse symlink folders #122

Merged
merged 4 commits into from
Mar 24, 2024
Merged

fix: Traverse symlink folders #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c05859
fix: Traverse symlink folders
joshhunt Mar 22, 2024
e6c022d
less newline
joshhunt Mar 22, 2024
8b6146b
fix the tests i missed
joshhunt Mar 22, 2024
619084d
test when fs.stat errors
joshhunt Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ function walkdir() {

if (dirent.isDirectory()) {
queue.push(nextpath);
} else if (dirent.isSymbolicLink()) {
// If it's a symlink, check if the symlink points to a directory
fs.stat(nextpath, function (err, stats) {
if (err) {
return cb(err);
}
Comment on lines +80 to +82
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure what to do about the error handling here. This cb tells the queue an error occured, but presumably dirents.forEach(processDirent); will continue processing, which could potentially error again...

Should it instead just throw the error and hope queue catches and emits it properly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out! It has me rethinking how things are queued, but I'll make those changes after I land this


if (stats.isDirectory()) {
queue.push(nextpath);
}
});
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/symlinks/file-a.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
file a
1 change: 1 addition & 0 deletions test/fixtures/symlinks/symlink-dest
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../whatsgoingon
69 changes: 68 additions & 1 deletion test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,18 @@ function suite(moduleName) {
base: dir + '/fixtures',
path: dir + '/fixtures/stuff/test.dmc',
},
{
cwd: dir,
base: dir + '/fixtures',
path: dir + '/fixtures/symlinks/symlink-dest/test.js',
},
{
cwd: dir,
base: dir + '/fixtures',
path:
dir +
'/fixtures/symlinks/symlink-dest/hey/isaidhey/whatsgoingon/test.txt',
},
];

var globs = [
Expand All @@ -344,12 +356,14 @@ function suite(moduleName) {
];

function assert(pathObjs) {
expect(pathObjs.length).toEqual(5);
expect(pathObjs.length).toEqual(7);
expect(pathObjs).toContainEqual(expected[0]);
expect(pathObjs).toContainEqual(expected[1]);
expect(pathObjs).toContainEqual(expected[2]);
expect(pathObjs).toContainEqual(expected[3]);
expect(pathObjs).toContainEqual(expected[4]);
expect(pathObjs).toContainEqual(expected[5]);
expect(pathObjs).toContainEqual(expected[6]);
}

stream.pipeline([globStream(globs, { cwd: dir }), concat(assert)], done);
Expand Down Expand Up @@ -736,6 +750,37 @@ function suite(moduleName) {
done
);
});

it('traverses symlinked directories', function (done) {
var expected = [
{
cwd: dir,
base: dir + '/fixtures/symlinks',
path: dir + '/fixtures/symlinks/file-a.txt',
},
{
cwd: dir,
base: dir + '/fixtures/symlinks',
path:
dir +
'/fixtures/symlinks/symlink-dest/hey/isaidhey/whatsgoingon/test.txt',
},
];

function assert(pathObjs) {
expect(pathObjs.length).toBe(2);
expect(pathObjs).toContainEqual(expected[0]);
expect(pathObjs).toContainEqual(expected[1]);
}

stream.pipeline(
[
globStream(['./fixtures/symlinks/**/*.txt'], { cwd: dir }),
concat(assert),
],
done
);
});
});

describe('options', function () {
Expand Down Expand Up @@ -1039,6 +1084,28 @@ function suite(moduleName) {
stream.pipeline([gs, concat()], assert);
});

it('destroys the stream if walker errors when following symlink', function (done) {
var expectedError = new Error('Stubbed error');

var gs = globStream('./fixtures/**/*.dmc', { cwd: dir });

function stubError(dirpath, cb) {
cb(expectedError);
}

var spy = sinon.spy(gs, 'destroy');
sinon.stub(fs, 'stat').callsFake(stubError);

function assert(err) {
sinon.restore();
expect(spy.called).toEqual(true);
expect(err).toBe(expectedError);
done();
}

stream.pipeline([gs, concat()], assert);
});

it('does not emit an error if stream is destroyed without an error', function (done) {
var gs = globStream('./fixtures/**/*.dmc', { cwd: dir });

Expand Down
Loading