Fix regression caused by spaces in the jsdoc binary path. jsdoc2md/jsdoc-to-markdown#307

Add Windows testing to CI

Author: 75lb

.github/workflows/node.js.yml

@@ -4,7 +4,7 @@ name: Node.js CI
 
 on:
   push:
-    branches: [ master ]
+    branches: [ master, next ]
   pull_request:
     branches: [ master ]
 
@@ -15,7 +15,7 @@ jobs:
 
     strategy:
       matrix:
-        os: [ubuntu-latest] # Remove windows as tests fail on `\r\n` style newlines
+        os: [ubuntu-latest, windows-latest]
         node-version: [12, 14, 16, 18, 20, 22, 23]
 
     steps:
@@ -26,5 +26,6 @@ jobs:
         node-version: ${{ matrix.node-version }}
         cache: 'npm'
     - run: npm install
-    - run: npm i -g @75lb/nature
+    - run: npm i -D @75lb/nature
     - run: npm run test:ci
+

dist/index.cjs

@@ -100,7 +100,7 @@ class JsdocCommand {
   }
 }
 
-const exec = util.promisify(cp.exec);
+util.promisify(cp.exec);
 
 class Explain extends JsdocCommand {
   async getOutput () {
@@ -120,18 +120,39 @@ class Explain extends JsdocCommand {
   }
 
   async _runJsdoc () {
-    const cmd = this.options.source.length
-      ? `node ${this.jsdocPath} ${toSpawnArgs(this.jsdocOptions).join(' ')} -X ${this.tempFileSet.files.join(' ')}`
-      : `node ${this.jsdocPath} ${toSpawnArgs(this.jsdocOptions).join(' ')} -X ${this.inputFileSet.files.join(' ')}`;
-
+    const jsdocArgs = [
+      this.jsdocPath,
+      ...toSpawnArgs(this.jsdocOptions),
+      '-X',
+      ...(this.options.source.length ? this.tempFileSet.files : this.inputFileSet.files)
+    ];
     let jsdocOutput = { stdout: '', stderr: '' };
+
+    const code = await new Promise((resolve, reject) => {
+      const handle = cp.spawn('node', jsdocArgs);
+      handle.stdout.setEncoding('utf8');
+      handle.stderr.setEncoding('utf8');
+      handle.stdout.on('data', chunk => {
+        jsdocOutput.stdout += chunk;
+      });
+      handle.stderr.on('data', chunk => {
+        jsdocOutput.stderr += chunk;
+      });
+      handle.on('exit', (code) => {
+        resolve(code);
+      });
+      handle.on('error', reject);
+    });
     try {
-      jsdocOutput = await exec(cmd, { maxBuffer: 1024 * 1024 * 100 }); /* 100MB */
-      const explainOutput = JSON.parse(jsdocOutput.stdout);
-      if (this.options.cache) {
-        await this.cache.write(this.cacheKey, explainOutput);
+      if (code > 0) {
+        throw new Error('jsdoc exited with non-zero code: ' + code)
+      } else {
+        const explainOutput = JSON.parse(jsdocOutput.stdout);
+        if (this.options.cache) {
+          await this.cache.write(this.cacheKey, explainOutput);
+        }
+        return explainOutput
       }
-      return explainOutput
     } catch (err) {
       const firstLineOfStdout = jsdocOutput.stdout.split(/\r?\n/)[0];
       const jsdocErr = new Error(jsdocOutput.stderr.trim() || firstLineOfStdout || 'Jsdoc failed.');

lib/explain.js

@@ -23,18 +23,39 @@ class Explain extends JsdocCommand {
   }
 
   async _runJsdoc () {
-    const cmd = this.options.source.length
-      ? `node ${this.jsdocPath} ${toSpawnArgs(this.jsdocOptions).join(' ')} -X ${this.tempFileSet.files.join(' ')}`
-      : `node ${this.jsdocPath} ${toSpawnArgs(this.jsdocOptions).join(' ')} -X ${this.inputFileSet.files.join(' ')}`
-
+    const jsdocArgs = [
+      this.jsdocPath,
+      ...toSpawnArgs(this.jsdocOptions),
+      '-X',
+      ...(this.options.source.length ? this.tempFileSet.files : this.inputFileSet.files)
+    ]
     let jsdocOutput = { stdout: '', stderr: '' }
+
+    const code = await new Promise((resolve, reject) => {
+      const handle = cp.spawn('node', jsdocArgs)
+      handle.stdout.setEncoding('utf8')
+      handle.stderr.setEncoding('utf8')
+      handle.stdout.on('data', chunk => {
+        jsdocOutput.stdout += chunk
+      })
+      handle.stderr.on('data', chunk => {
+        jsdocOutput.stderr += chunk
+      })
+      handle.on('exit', (code) => {
+        resolve(code)
+      })
+      handle.on('error', reject)
+    })
     try {
-      jsdocOutput = await exec(cmd, { maxBuffer: 1024 * 1024 * 100 }) /* 100MB */
-      const explainOutput = JSON.parse(jsdocOutput.stdout)
-      if (this.options.cache) {
-        await this.cache.write(this.cacheKey, explainOutput)
+      if (code > 0) {
+        throw new Error('jsdoc exited with non-zero code: ' + code)
+      } else {
+        const explainOutput = JSON.parse(jsdocOutput.stdout)
+        if (this.options.cache) {
+          await this.cache.write(this.cacheKey, explainOutput)
+        }
+        return explainOutput
       }
-      return explainOutput
     } catch (err) {
       const firstLineOfStdout = jsdocOutput.stdout.split(/\r?\n/)[0]
       const jsdocErr = new Error(jsdocOutput.stderr.trim() || firstLineOfStdout || 'Jsdoc failed.')

package-lock.json

@@ -26,7 +26,7 @@
   },
   "scripts": {
     "test": "npm run dist && npm run test:ci",
-    "test:ci": "75lb-nature test-runner test/*.js",
+    "test:ci": "75lb-nature test-runner test/caching.js test/explain.js test/render.js",
     "dist": "75lb-nature cjs-build index.js",
     "docs": "75lb-nature jsdoc2md index.js lib/*.js > docs/api.md"
   },

package.json

@@ -11,12 +11,13 @@ test.set('.explain({ files, cache: true  })', async function () {
   const f = new Fixture('class-all')
   jsdoc.cache.dir = 'tmp/test/cache1'
   await jsdoc.cache.clear()
-  const output = await jsdoc.explain({ files: f.sourcePath, cache: true })
-  const cachedFiles = readdirSync(jsdoc.cache.dir)
-    .map(file => path.resolve(jsdoc.cache.dir, file))
+  let output = await jsdoc.explain({ files: f.sourcePath, cache: true })
+  output = Fixture.normaliseNewLines(output)
+  const cachedFiles = readdirSync(jsdoc.cache.dir).map(file => path.resolve(jsdoc.cache.dir, file))
   a.equal(cachedFiles.length, 1)
   a.deepEqual(output, f.getExpectedOutput(output))
-  const cachedData = JSON.parse(readFileSync(cachedFiles[0], 'utf8'))
+  let cachedData = JSON.parse(readFileSync(cachedFiles[0], 'utf8'))
+  cachedData = Fixture.normaliseNewLines(cachedData)
   Fixture.removeFileSpecificData(cachedData)
   a.deepEqual(
     cachedData,

test/caching.js

@@ -7,13 +7,15 @@ const [test, only, skip] = [new Map(), new Map(), new Map()]
 
 test.set('.explain({ files })', async function () {
   const f = new Fixture('class-all')
-  const output = await jsdoc.explain({ files: f.sourcePath })
+  let output = await jsdoc.explain({ files: f.sourcePath })
+  output = Fixture.normaliseNewLines(output)
   a.deepEqual(output, f.getExpectedOutput(output))
 })
 
 test.set('.explain({ source })', async function () {
   const f = new Fixture('class-all')
-  const output = await jsdoc.explain({ source: f.getSource() })
+  let output = await jsdoc.explain({ source: f.getSource() })
+  output = Fixture.normaliseNewLines(output)
   a.deepEqual(output, f.getExpectedOutput(output))
 })
 
@@ -47,4 +49,12 @@ test.set('.explain({ files }): files is empty', async function () {
   )
 })
 
+test.set('Spaces in jsdoc command path', async function () {
+  process.env.JSDOC_PATH = 'test/fixture/folder with spaces/fake-jsdoc.js'
+  const f = new Fixture('class-all')
+  let output = await jsdoc.explain({ files: f.sourcePath })
+  a.equal(output.length, 4)
+  process.env.JSDOC_PATH = ''
+})
+
 export { test, only, skip }

test/explain.js

@@ -0,0 +1 @@
+console.log(JSON.stringify(process.argv))

test/fixture/folder with spaces/fake-jsdoc.js

@@ -44,6 +44,16 @@ class Fixture {
       }
     })
   }
+
+  static normaliseNewLines (doclets) {
+    const input = JSON.stringify(doclets, null, '  ')
+    /* Normalise all newlines to posix style to avoid differences while testing on Windows */
+    let result = input.replace(/\\r?\\n/gm, '\\n')
+    /* Additional check for naked \r characters created by jsdoc */
+    /* See: https://github.com/jsdoc2md/dmd/issues/102 */
+    result = result.replace(/\\r(?!\\n)/g, '\\n')
+    return JSON.parse(result)
+  }
 }
 
 export default Fixture