fix: convert error output to be consistent cross-platform (#684)

* fix: convert error output to be consistent cross-platform

The error output produced by `shell.error()` or `result.stderr` should
not be inconsistent between platforms. This ensures that path separators
are always printed by ShellJS as `/` instead of as `\` on Windows. This
should allow scripts using ShellJS to be more consistent cross-platform.

We were not previously relying on error output to always be consistent--
only checking its truthiness. Since this was not part of our tested API,
it should be reasonable to change this and release in a patch.

Fixes #681

* Fix broken pushd test case

* Fix TODO in a test case

src/common.js

@@ -80,6 +80,17 @@ function log() {
 }
 exports.log = log;
 
+// Converts strings to be equivalent across all platforms. Primarily responsible
+// for making sure we use '/' instead of '\' as path separators, but this may be
+// expanded in the future if necessary
+function convertErrorOutput(msg) {
+  if (typeof msg !== 'string') {
+    throw new TypeError('input must be a string');
+  }
+  return msg.replace(/\\/g, '/');
+}
+exports.convertErrorOutput = convertErrorOutput;
+
 // Shows error message. Throws if config.fatal is true
 function error(msg, _code, options) {
   // Validate input
@@ -105,7 +116,7 @@ function error(msg, _code, options) {
 
   if (!state.errorCode) state.errorCode = options.code;
 
-  var logEntry = options.prefix + msg;
+  var logEntry = convertErrorOutput(options.prefix + msg);
   state.error = state.error ? state.error + '\n' : '';
   state.error += logEntry;
 

test/common.js

@@ -47,10 +47,50 @@ test('parseOptions (invalid type)', t => {
   });
 });
 
+test('convertErrorOutput: no args', t => {
+  t.throws(() => {
+    common.convertErrorOutput();
+  }, TypeError);
+});
+
+test('convertErrorOutput: input must be a vanilla string', t => {
+  t.throws(() => {
+    common.convertErrorOutput(3);
+  }, TypeError);
+
+  t.throws(() => {
+    common.convertErrorOutput({});
+  }, TypeError);
+});
+
 //
 // Valids
 //
 
+//
+// common.convertErrorOutput()
+//
+test('convertErrorOutput: nothing to convert', t => {
+  const input = 'hello world';
+  const result = common.convertErrorOutput(input);
+  t.is(result, input);
+});
+
+test('convertErrorOutput: does not change forward slash', t => {
+  const input = 'dir/sub/file.txt';
+  const result = common.convertErrorOutput(input);
+  t.is(result, input);
+});
+
+test('convertErrorOutput: changes backslashes to forward slashes', t => {
+  const input = 'dir\\sub\\file.txt';
+  const result = common.convertErrorOutput(input);
+  t.is(result, 'dir/sub/file.txt');
+});
+
+//
+// common.expand()
+//
 test('single file, array syntax', t => {
   const result = common.expand(['resources/file1.txt']);
   t.falsy(shell.error());

test/mv.js

@@ -96,8 +96,7 @@ test('-n option with a directory as the destination', t => {
   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/);
+  t.is(result.stderr, 'mv: dest file already exists: cp/file1');
 });
 
 test('-f is the default behavior', t => {

test/pushd.js

@@ -273,7 +273,8 @@ test('Push invalid directory', t => {
   shell.pushd('does/not/exist');
   t.is(
     shell.error(),
-    'pushd: no such file or directory: ' + path.resolve('.', 'does/not/exist')
+    `pushd: no such file or directory: ${path.resolve('.', 'does/not/exist')
+      .replace(/\\/g, '/')}`
   );
   t.is(process.cwd(), oldCwd);
 });