From 35016ce1e7cb2a9be4ba3d5ea141cb89a9eae2d2 Mon Sep 17 00:00:00 2001
From: Nate Fischer <ntfschr@gmail.com>
Date: Sun, 5 Mar 2017 02:28:47 -0800
Subject: [PATCH] test: add misc. tests to improve coverage (#673)

No change in production logic.

This adds missing tests to improve test coverage. This does not change
ShellJS behavior at all--all test cases are testing already-working
functionality.

One test case has been renamed for clarity.

For the "omit directory if missing recursive flag" case, we were
actually already testing that in another case, but we were testing
multiple things in that test case. It's better to test this one error
condition explicitly in its own case.

When adding real tests for `parseOptions()`, we need to explicitly clear
`common.state.error` because we're testing an internal function, not a
wrapped command.

Partial fix for #671
---
 test/common.js | 25 +++++++++++++++++++++++++
 test/cp.js     | 37 ++++++++++++++++++++++++++++++++++++-
 test/mv.js     | 21 ++++++++++++---------
 test/rm.js     |  7 +++++++
 test/toEnd.js  |  6 ++++++
 5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/test/common.js b/test/common.js
index 2bb4a76..bc2c4da 100644
--- a/test/common.js
+++ b/test/common.js
@@ -6,6 +6,11 @@ import utils from './utils/utils';
 
 shell.config.silent = true;
 
+test.beforeEach(() => {
+  common.state.error = null;
+  common.state.errorCode = 0;
+});
+
 //
 // Invalids
 //
@@ -22,6 +27,26 @@ test('should be a list', t => {
   }, TypeError);
 });
 
+test('parseOptions (invalid option in options object)', t => {
+  t.throws(() => {
+    common.parseOptions({ q: 'some string value' }, {
+      R: 'recursive',
+      f: 'force',
+      r: 'reverse',
+    });
+  });
+});
+
+test('parseOptions (invalid type)', t => {
+  t.throws(() => {
+    common.parseOptions(12, {
+      R: 'recursive',
+      f: 'force',
+      r: 'reverse',
+    });
+  });
+});
+
 //
 // Valids
 //
diff --git a/test/cp.js b/test/cp.js
index 4cb16e5..74c8dd8 100644
--- a/test/cp.js
+++ b/test/cp.js
@@ -71,7 +71,7 @@ test('source does not exist', t => {
   t.is(result.stderr, 'cp: no such file or directory: asdfasdf');
 });
 
-test('sources does not exist', t => {
+test('multiple sources do not exist', t => {
   const result = shell.cp('asdfasdf1', 'asdfasdf2', t.context.tmp);
   t.truthy(shell.error());
   t.is(result.code, 1);
@@ -112,6 +112,32 @@ test('dest already exists', t => {
   t.is(shell.cat('resources/file2').toString(), oldContents);
 });
 
+test('-nR does not overwrite an existing file at the destination', t => {
+  // Create tmp/new/cp/a
+  const dest = `${t.context.tmp}/new/cp`;
+  shell.mkdir('-p', dest);
+  const oldContents = 'original content';
+  shell.ShellString(oldContents).to(`${dest}/a`);
+
+  // Attempt to overwrite /tmp/new/cp/ with resources/cp/
+  const result = shell.cp('-nR', 'resources/cp/', `${t.context.tmp}/new/`);
+  t.falsy(shell.error());
+  t.is(result.code, 0);
+  t.falsy(result.stderr);
+  t.is(shell.cat(`${dest}/a`).toString(), oldContents);
+});
+
+test('-n does not overwrite an existing file if the destination is a directory', t => {
+  const oldContents = 'original content';
+  shell.cp('resources/file1', `${t.context.tmp}`);
+  new common.ShellString(oldContents).to(`${t.context.tmp}/file1`);
+  const result = shell.cp('-n', 'resources/file1', `${t.context.tmp}`);
+  t.falsy(shell.error());
+  t.is(result.code, 0);
+  t.falsy(result.stderr);
+  t.is(shell.cat(`${t.context.tmp}/file1`).toString(), oldContents);
+});
+
 test('-f by default', t => {
   shell.cp('resources/file2', 'resources/copyfile2');
   const result = shell.cp('resources/file1', 'resources/file2'); // dest already exists
@@ -215,6 +241,15 @@ test('recursive, with regular files', t => {
   t.truthy(fs.existsSync(`${t.context.tmp}/file2`));
 });
 
+test('omit directory if missing recursive flag', t => {
+  const result = shell.cp('resources/cp', t.context.tmp);
+  t.is(shell.error(), "cp: omitting directory 'resources/cp'");
+  t.is(result.stderr, "cp: omitting directory 'resources/cp'");
+  t.is(result.code, 1);
+  t.falsy(fs.existsSync(`${t.context.tmp}/file1`));
+  t.falsy(fs.existsSync(`${t.context.tmp}/file2`));
+});
+
 test('recursive, nothing exists', t => {
   const result = shell.cp('-R', 'resources/cp', t.context.tmp);
   t.falsy(shell.error());
diff --git a/test/mv.js b/test/mv.js
index dcf1e22..9d3fbaf 100644
--- a/test/mv.js
+++ b/test/mv.js
@@ -91,6 +91,15 @@ test('-n is no-force/no-clobber', t => {
   t.is(result.stderr, 'mv: dest file already exists: file2');
 });
 
+test('-n option with a directory as the destination', t => {
+  shell.cp('file1', 'cp'); // copy it so we're sure it's already there
+  const result = shell.mv('-n', 'file1', 'cp');
+  t.truthy(shell.error());
+  t.is(result.code, 1);
+  // TODO(nate): make this an equals comparison once issue #681 is resolved
+  t.regex(result.stderr, /mv: dest file already exists: cp.file1/);
+});
+
 test('-f is the default behavior', t => {
   const result = shell.mv('file1', 'file2'); // dest already exists (but that's ok)
   t.falsy(shell.error());
@@ -153,19 +162,13 @@ test('one source', t => {
 });
 
 test('two sources', t => {
-  shell.rm('-rf', 't');
-  shell.mkdir('-p', 't');
-  let result = shell.mv('file1', 'file2', 't');
+  const result = shell.mv('file1', 'file2', 'cp');
   t.falsy(shell.error());
   t.is(result.code, 0);
   t.falsy(fs.existsSync('file1'));
   t.falsy(fs.existsSync('file2'));
-  t.truthy(fs.existsSync('t/file1'));
-  t.truthy(fs.existsSync('t/file2'));
-  result = shell.mv('t/*', '.'); // revert
-  t.is(result.code, 0);
-  t.truthy(fs.existsSync('file1'));
-  t.truthy(fs.existsSync('file2'));
+  t.truthy(fs.existsSync('cp/file1'));
+  t.truthy(fs.existsSync('cp/file2'));
 });
 
 test('two sources, array style', t => {
diff --git a/test/rm.js b/test/rm.js
index cdb3e92..1d88a88 100644
--- a/test/rm.js
+++ b/test/rm.js
@@ -35,6 +35,13 @@ 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 => {
+  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');
+});
+
 test('only an option', t => {
   const result = shell.rm('-f');
   t.truthy(shell.error());
diff --git a/test/toEnd.js b/test/toEnd.js
index 323e4a8..cdb7fb6 100644
--- a/test/toEnd.js
+++ b/test/toEnd.js
@@ -30,6 +30,12 @@ test('missing file argument', t => {
   t.truthy(shell.error());
 });
 
+test('cannot write to a non-existent directory', t => {
+  t.falsy(fs.existsSync('/asdfasdf')); // sanity check
+  shell.ShellString('hello world').toEnd('/asdfasdf/file');
+  t.truthy(shell.error());
+});
+
 //
 // Valids
 //