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

Add tests for rm removing symlinks #849

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add tests for rm removing symlinks #849

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
64a682e
test(rm): add tests for removing symlinks
freitagbr May 10, 2018
c678839
test(rm): skip symlink test on windows
freitagbr May 10, 2018
1438bf1
Rename and move test, add assertions
freitagbr May 11, 2018
4167033
Add tests for rm -r/-f on links to dirs
freitagbr May 11, 2018
042d17c
Properly handle rm -r on links to dirs with a trailing slash
freitagbr Jul 12, 2018
4639ca9
Rephrase comment for clarity
freitagbr Jul 18, 2018
44ec920
Use -r in -r test
freitagbr Jul 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 71 additions & 3 deletions test/rm.js
View file
Open in desktop
Expand Up @@ -35,11 +35,14 @@ test('file does not exist', t => {
t.is(result.stderr, 'rm: no such file or directory: asdfasdf');
});

test('cannot delete a directoy without recursive flag', t => {
test('cannot delete a directory without recursive flag', t => {
const result = shell.rm(`${t.context.tmp}/rm`);
t.truthy(shell.error());
t.is(result.code, 1);
t.is(result.stderr, 'rm: path is a directory');
t.truthy(fs.existsSync(`${t.context.tmp}/rm`));
const contents = fs.readdirSync(t.context.tmp);
t.true(contents.length > 0);
});

test('only an option', t => {
Expand All @@ -57,6 +60,24 @@ test('invalid option', t => {
t.is(result.stderr, 'rm: option not recognized: @');
});

test(
'cannot remove a symbolic link to a directory with trailing slash without -r flag',
t => {
utils.skipOnWin(t, () => {
// the trailing slash signifies that we want to delete the source
// directory and its contents, which can only be done with the -r flag
Copy link
Member

Choose a reason for hiding this comment

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

to delete the source directory and its contents

Can you rephrase this as "delete the contents of the source directory, without removing the source directory itself or the link"?

const result = shell.rm(`${t.context.tmp}/rm/link_to_a_dir/`);
t.truthy(shell.error());
t.is(result.code, 1);
t.is(result.stderr, 'rm: path is a directory');
t.truthy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
const contents = fs.readdirSync(`${t.context.tmp}/rm/a_dir`);
t.true(contents.length > 0);
});
}
);

//
// Valids
//
Expand Down Expand Up @@ -259,27 +280,74 @@ test(
}
);

test('remove symbolic link to a file', t => {
t.truthy(shell.test('-L', `${t.context.tmp}/link`));
const result = shell.rm(`${t.context.tmp}/link`);
t.falsy(shell.error());
t.is(result.code, 0);
t.falsy(shell.test('-L', `${t.context.tmp}/link`));
t.falsy(fs.existsSync(`${t.context.tmp}/link`));
t.truthy(fs.existsSync(`${t.context.tmp}/file1`));
});

test('remove symbolic link to a dir', t => {
t.truthy(shell.test('-L', `${t.context.tmp}/rm/link_to_a_dir`));
const result = shell.rm(`${t.context.tmp}/rm/link_to_a_dir`);
t.falsy(shell.error());
t.is(result.code, 0);
t.falsy(shell.test('-L', `${t.context.tmp}/rm/link_to_a_dir`));
Copy link
Member

Choose a reason for hiding this comment

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

I think this is already covered by the next line? Or, does existsSync follow links? If there's a reason, can you add a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

existsSync follows links. This pattern is found in several of the tests here, I will clarify them with comments.

t.falsy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
});

test('rm -rf on a symbolic link to a dir deletes its contents', t => {
test('rm -r, symlink to a dir, trailing slash', t => {
utils.skipOnWin(t, () => {
const result = shell.rm('-r', `${t.context.tmp}/rm/link_to_a_dir/`);
t.truthy(shell.error());
t.is(result.code, 1);
// Both the link and original dir should remain, but contents are deleted
t.truthy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
t.falsy(fs.existsSync(`${t.context.tmp}/rm/a_dir/a_file`));
});
});

test('rm -rf, symlink to a dir, trailing slash', t => {
utils.skipOnWin(t, () => {
const result = shell.rm('-rf', `${t.context.tmp}/rm/link_to_a_dir/`);
t.falsy(shell.error());
t.is(result.code, 0);

// Both the link and original dir should remain, but contents are deleted
t.truthy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
t.falsy(fs.existsSync(`${t.context.tmp}/rm/a_dir/a_file`));
});
});

test('rm -r, symlink to a dir, no trailing slash', t => {
utils.skipOnWin(t, () => {
const result = shell.rm('-rf', `${t.context.tmp}/rm/link_to_a_dir`);
Copy link
Member

Choose a reason for hiding this comment

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

Should be -r not -rf

t.falsy(shell.error());
t.is(result.code, 0);
// The link should be deleted, but the dir and contents remain
t.falsy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir/a_file`));
});
});

test('rm -rf, symlink to a dir, no trailing slash', t => {
utils.skipOnWin(t, () => {
const result = shell.rm('-rf', `${t.context.tmp}/rm/link_to_a_dir`);
t.falsy(shell.error());
t.is(result.code, 0);
// The link should be deleted, but the dir and contents remain
t.falsy(fs.existsSync(`${t.context.tmp}/rm/link_to_a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir`));
t.truthy(fs.existsSync(`${t.context.tmp}/rm/a_dir/a_file`));
});
});

test('remove broken symbolic link', t => {
utils.skipOnWin(t, () => {
t.truthy(shell.test('-L', `${t.context.tmp}/rm/fake.lnk`));
Expand Down