Merge pull request #180 from jamesramsay/optional-args

Fix: sync API incorrectly required `relativePath` option (fixes #178)
jamesramsay · Jan 31, 2016 · 489b296 · 489b296
2 parents 434c298 + c5818fc
commit 489b296
Show file tree

Hide file tree

Showing 9 changed files with 254 additions and 103 deletions.
diff --git a/src/hercule.js b/src/hercule.js
@@ -14,9 +14,12 @@ const LOG_LEVELS = {
   50: 'error',
 };
 
+function relog(log, message) {
+  const msg = message.msg;
+  const level = message.level;
+  const body = _.omit(message, LOG_OMIT);
 
-function relog(log, level, body, message) {
-  log[LOG_LEVELS[level]](body, message);
+  log[LOG_LEVELS[level]](body, msg);
 }
 
 export const TranscludeStream = Transcluder;
@@ -65,44 +68,43 @@ export function transcludeFile(...args) {
 }
 
 
-export function transcludeFileSync(input, { relativePath }, log) {
-  const options = {
-    cwd: __dirname,
-    timeout: SYNC_TIMEOUT,
-  };
-  const args = [input, '--reporter', 'json-err'];
-  const result = childProcess.spawnSync('../bin/hercule', args, options);
+export function transcludeFileSync(...args) {
+  const input = args.shift();
+  const [options, log] = args;
+
+  const syncOptions = { cwd: __dirname, timeout: SYNC_TIMEOUT };
+  const syncArgs = [input, '--reporter', 'json-err'];
+
+  _.each(options, (optionValue, optionName) => {
+    syncArgs.push(`--${optionName}`, `${optionValue}`);
+  });
+
+  const result = childProcess.spawnSync('../bin/hercule', syncArgs, syncOptions);
   const outputContent = result.stdout.toString();
   const outputLogs = result.stderr.toString().split('\n');
 
-  _.compact(outputLogs).map(JSON.parse).forEach((message) => {
-    const msg = message.msg;
-    const level = message.level;
-    const body = _.omit(message, LOG_OMIT);
-    relog(log, level, body, msg);
-  });
+  _.compact(outputLogs).map(JSON.parse).forEach((message) => relog(log, message));
 
   return outputContent;
 }
 
 
-export function transcludeStringSync(input, { relativePath }, log) {
-  const options = {
-    input,
-    cwd: __dirname,
-    timeout: SYNC_TIMEOUT,
-  };
-  const args = ['--relative', relativePath, '--reporter', 'json-err'];
-  const result = childProcess.spawnSync('../bin/hercule', args, options);
+export function transcludeStringSync(...args) {
+  const input = args.shift();
+  const [options, log] = args;
+
+  const syncOptions = { input, cwd: __dirname, timeout: SYNC_TIMEOUT };
+  const syncArgs = ['--reporter', 'json-err'];
+
+  _.each(options, (optionValue, optionName) => {
+    syncArgs.push(`--${optionName}`, `${optionValue}`);
+  });
+
+  const result = childProcess.spawnSync('../bin/hercule', syncArgs, syncOptions);
   const outputContent = result.stdout.toString();
   const outputLogs = result.stderr.toString().split('\n');
 
-  _.compact(outputLogs).map(JSON.parse).forEach((message) => {
-    const msg = message.msg;
-    const level = message.level;
-    const body = _.omit(message, LOG_OMIT);
-    relog(log, level, body, msg);
-  });
+  _.compact(outputLogs).map(JSON.parse).forEach((message) => relog(log, message));
 
   return outputContent;
 }
diff --git a/src/main.js b/src/main.js
@@ -29,7 +29,7 @@ const parser = dashdash.createParser({
       helpArg: 'FILE',
     },
     {
-      names: ['relative', 'r'],
+      names: ['relativePath', 'r'],
       type: 'string',
       help: 'Relative path. stdin will be parsed relative to this path.',
     },
@@ -73,9 +73,10 @@ function main() {
   if (opts._args.length === 0) {
     // Reading input from stdin
     inputStream = process.stdin;
-    options.relativePath = opts.relative;
+    options.relativePath = opts.relativePath;
   } else {
     // Reading input from file
+    // TODO: handle file error!
     inputStream = fs.createReadStream(opts._args[0], { encoding: 'utf8' });
     options.relativePath = path.dirname(opts._args[0]);
   }

diff --git a/test/bats b/test/bats
@@ -13,7 +13,7 @@
 }
 
 @test "transcluding stdin" {
-  result="$(cat ./test/fixtures/local-link/index.md | ./bin/hercule --relative ./test/fixtures/local-link)"
+  result="$(cat ./test/fixtures/local-link/index.md | ./bin/hercule --relativePath ./test/fixtures/local-link)"
   expected="Jackdaws love my big sphinx of quartz."
   [ "$result" == "$expected" ]
 }
diff --git a/test/integration/transcludeFileSync.js b/test/integration/transcludeFileSync.js
@@ -13,45 +13,45 @@ let minor;
 [major, minor] = process.versions.node.split('.');
 
 if (major < 1 && minor < 12) {
-  test(`synchronous support not available < 0.12`, (t) => {
+  test.only(`synchronous support not available < 0.12`, (t) => {
     t.pass();
   });
-} else {
-  test.beforeEach((t) => {
-    t.context.logOutput = [];
-    t.context.logStream = through2.obj();
-
-    t.context.log = bunyan.createLogger({
-      name: 'hercule',
-      streams: [{
-        stream: t.context.logStream,
-      }],
-    });
-
-    t.context.logStream.on('readable', function read() {
-      let message = null;
-      while ((message = this.read()) !== null) {
-        message = _.pick(JSON.parse(message), 'name', 'msg', 'link', 'level');
-
-        // Make paths relatice to process for testing purposes
-        if (message.link && message.link.href) {
-          message.link.href = path.relative(process.cwd(), message.link.href);
-        }
-
-        t.context.logOutput.push(message);
+}
+
+test.beforeEach((t) => {
+  t.context.logOutput = [];
+  t.context.logStream = through2.obj();
+
+  t.context.log = bunyan.createLogger({
+    name: 'hercule',
+    streams: [{
+      stream: t.context.logStream,
+    }],
+  });
+
+  t.context.logStream.on('readable', function read() {
+    let message = null;
+    while ((message = this.read()) !== null) {
+      message = _.pick(JSON.parse(message), 'name', 'msg', 'link', 'level');
+
+      // Make paths relatice to process for testing purposes
+      if (message.link && message.link.href) {
+        message.link.href = path.relative(process.cwd(), message.link.href);
       }
-    });
+
+      t.context.logOutput.push(message);
+    }
   });
+});
 
 
-  _.forEach((fixtures.fixtures), (fixture) => {
-    // Exclude http tests because mocking won't cover sync sub-process
-    if (_.includes(['http-link', 'http-deep-nesting'], fixture.name)) return;
+_.forEach((fixtures.fixtures), (fixture) => {
+  // Exclude http tests because mocking won't cover sync sub-process
+  if (_.includes(['http-link', 'http-deep-nesting'], fixture.name)) return;
 
-    test(`should transclude ${fixture.name}`, (t) => {
-      const output = transcludeFileSync(fixture.inputFile, {}, t.context.log);
-      t.same(output, fixture.expectedOutput);
-      t.same(t.context.logOutput, fixture.expectedLogOutput);
-    });
+  test(`should transclude ${fixture.name}`, (t) => {
+    const output = transcludeFileSync(fixture.inputFile, {}, t.context.log);
+    t.same(output, fixture.expectedOutput);
+    t.same(t.context.logOutput, fixture.expectedLogOutput);
   });
-}
+});
diff --git a/test/integration/transcludeStringSync.js b/test/integration/transcludeStringSync.js
@@ -13,48 +13,48 @@ let minor;
 [major, minor] = process.versions.node.split('.');
 
 if (major < 1 && minor < 12) {
-  test(`synchronous support not available < 0.12`, (t) => {
+  test.only(`synchronous support not available < 0.12`, (t) => {
     t.pass();
   });
-} else {
-  test.beforeEach((t) => {
-    t.context.logOutput = [];
-    t.context.logStream = through2.obj();
-
-    t.context.log = bunyan.createLogger({
-      name: 'hercule',
-      streams: [{
-        stream: t.context.logStream,
-      }],
-    });
-
-    t.context.logStream.on('readable', function read() {
-      let message = null;
-      while ((message = this.read()) !== null) {
-        message = _.pick(JSON.parse(message), 'name', 'msg', 'link', 'level');
-
-        // Make paths relatice to process for testing purposes
-        if (message.link && message.link.href) {
-          message.link.href = path.relative(process.cwd(), message.link.href);
-        }
-
-        t.context.logOutput.push(message);
+}
+
+test.beforeEach((t) => {
+  t.context.logOutput = [];
+  t.context.logStream = through2.obj();
+
+  t.context.log = bunyan.createLogger({
+    name: 'hercule',
+    streams: [{
+      stream: t.context.logStream,
+    }],
+  });
+
+  t.context.logStream.on('readable', function read() {
+    let message = null;
+    while ((message = this.read()) !== null) {
+      message = _.pick(JSON.parse(message), 'name', 'msg', 'link', 'level');
+
+      // Make paths relatice to process for testing purposes
+      if (message.link && message.link.href) {
+        message.link.href = path.relative(process.cwd(), message.link.href);
       }
-    });
+
+      t.context.logOutput.push(message);
+    }
   });
+});
 
 
-  _.forEach((fixtures.fixtures), (fixture) => {
-    // Exclude http tests because mocking won't cover sub-process
-    if (_.includes(['http-link', 'http-deep-nesting'], fixture.name)) return;
+_.forEach((fixtures.fixtures), (fixture) => {
+  // Exclude http tests because mocking won't cover sub-process
+  if (_.includes(['http-link', 'http-deep-nesting'], fixture.name)) return;
 
-    test(`should transclude ${fixture.name}`, (t) => {
-      const options = {
-        relativePath: path.resolve(__dirname, '../fixtures', fixture.name),
-      };
-      const output = transcludeStringSync(fixture.input, options, t.context.log);
-      t.same(output, fixture.expectedOutput);
-      t.same(t.context.logOutput, fixture.expectedLogOutput);
-    });
+  test(`should transclude ${fixture.name}`, (t) => {
+    const options = {
+      relativePath: path.resolve(__dirname, '../fixtures', fixture.name),
+    };
+    const output = transcludeStringSync(fixture.input, options, t.context.log);
+    t.same(output, fixture.expectedOutput);
+    t.same(t.context.logOutput, fixture.expectedLogOutput);
   });
-}
+});
diff --git a/test/units/transcludeFile.js b/test/units/transcludeFile.js
@@ -0,0 +1,35 @@
+import test from 'ava';
+import path from 'path';
+
+import { transcludeFile } from '../../lib/hercule';
+
+test.cb(`should transclude with only required arguments`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  transcludeFile(input, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});
+
+test.cb(`should transclude with optional relativePath argument`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  transcludeFile(input, { relativePath: 'test' }, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});
+
+test.cb(`should transclude with optional log handler`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  const logger = {
+    error: () => t.fail(),
+    warn: () => t.fail(),
+  };
+  transcludeFile(input, null, logger, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});
diff --git a/test/units/transcludeFileSync.js b/test/units/transcludeFileSync.js
@@ -0,0 +1,40 @@
+import test from 'ava';
+import path from 'path';
+
+import { transcludeFileSync } from '../../lib/hercule';
+
+let major;
+let minor;
+
+[major, minor] = process.versions.node.split('.');
+
+if (major < 1 && minor < 12) {
+  test.only(`synchronous support not available < 0.12`, (t) => {
+    t.pass();
+  });
+}
+
+test(`should transclude with only required arguments`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  const output = transcludeFileSync(input);
+  t.same(output, expected);
+});
+
+test(`should transclude with optional relativePath argument`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  const output = transcludeFileSync(input, { relativePath: 'test' });
+  t.same(output, expected);
+});
+
+test(`should transclude with optional log handler`, (t) => {
+  const input = path.join(__dirname, '../fixtures/no-link/index.md');
+  const expected = 'The quick brown fox jumps over the lazy dog.\n';
+  const logger = {
+    error: () => t.fail(),
+    warn: () => t.fail(),
+  };
+  const output = transcludeFileSync(input, null, logger);
+  t.same(output, expected);
+});
diff --git a/test/units/transcludeString.js b/test/units/transcludeString.js
@@ -0,0 +1,34 @@
+import test from 'ava';
+
+import { transcludeString } from '../../lib/hercule';
+
+test.cb(`should transclude with only required arguments`, (t) => {
+  const input = 'The quick brown fox jumps over the lazy dog.';
+  const expected = 'The quick brown fox jumps over the lazy dog.';
+  transcludeString(input, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});
+
+test.cb(`should transclude with optional relativePath argument`, (t) => {
+  const input = 'The quick brown fox jumps over the lazy dog.';
+  const expected = 'The quick brown fox jumps over the lazy dog.';
+  transcludeString(input, { relativePath: 'test' }, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});
+
+test.cb(`should transclude with optional log handler`, (t) => {
+  const input = 'The quick brown fox jumps over the lazy dog.';
+  const expected = 'The quick brown fox jumps over the lazy dog.';
+  const logger = {
+    error: () => t.fail(),
+    warn: () => t.fail(),
+  };
+  transcludeString(input, null, logger, (output) => {
+    t.same(output, expected);
+    t.end();
+  });
+});