tools: only fetch previous versions when necessary

Refactor the logic for working out the previous versions of Node.js for
the API documentation so that the parsing (including the potential https
get) happens at most once per build (as opposed to the current once per
generated API doc).

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: https://github.com/nodejs/node/pull/32518
Fixes: https://github.com/nodejs/node/issues/32512
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

2 files changed, 65 insertions, 39 deletions

diff --git a/test/doctool/test-doctool-html.js b/test/doctool/test-doctool-html.js
index ee8099f8cd8..0ffa9f91490 100644
--- a/test/doctool/test-doctool-html.js
+++ b/test/doctool/test-doctool-html.js
@@ -36,7 +36,7 @@ const testLinksMapper = {
   }
 };
 
-async function toHTML({ input, filename, nodeVersion }) {
+function toHTML({ input, filename, nodeVersion, versions }) {
   const content = unified()
     .use(replaceLinks, { filename, linksMapper: testLinksMapper })
     .use(markdown)
@@ -49,7 +49,7 @@ async function toHTML({ input, filename, nodeVersion }) {
     .use(htmlStringify)
     .processSync(input);
 
-  return html.toHTML({ input, content, filename, nodeVersion });
+  return html.toHTML({ input, content, filename, nodeVersion, versions });
 }
 
 // Test data is a list of objects with two properties.
@@ -129,6 +129,16 @@ const testData = [
 ];
 
 const spaces = /\s/g;
+const versions = [
+  { num: '10.x', lts: true },
+  { num: '9.x' },
+  { num: '8.x' },
+  { num: '7.x' },
+  { num: '6.x' },
+  { num: '5.x' },
+  { num: '4.x' },
+  { num: '0.12.x' },
+  { num: '0.10.x' }];
 
 testData.forEach(({ file, html }) => {
   // Normalize expected data by stripping whitespace.
@@ -136,9 +146,10 @@ testData.forEach(({ file, html }) => {
 
   readFile(file, 'utf8', common.mustCall(async (err, input) => {
     assert.ifError(err);
-    const output = await toHTML({ input: input,
-                                  filename: 'foo',
-                                  nodeVersion: process.version });
+    const output = toHTML({ input: input,
+                            filename: 'foo',
+                            nodeVersion: process.version,
+                            versions: versions });
 
     const actual = output.replace(spaces, '');
     // Assert that the input stripped of all whitespace contains the
diff --git a/test/doctool/test-doctool-versions.js b/test/doctool/test-doctool-versions.js
index 8bb4f81c795..a37ead7c0ad 100644
--- a/test/doctool/test-doctool-versions.js
+++ b/test/doctool/test-doctool-versions.js
@@ -2,8 +2,14 @@
 
 require('../common');
 const assert = require('assert');
+const { spawnSync } = require('child_process');
+const fs = require('fs');
+const path = require('path');
+const tmpdir = require('../common/tmpdir');
 const util = require('util');
-const { versions } = require('../../tools/doc/versions.js');
+
+const debuglog = util.debuglog('test');
+const versionsTool = path.join('../../tools/doc/versions.js');
 
 // At the time of writing these are the minimum expected versions.
 // New versions of Node.js do not have to be explicitly added here.
@@ -21,39 +27,48 @@ const expected = [
   '0.10.x',
 ];
 
-async function test() {
-  const vers = await versions();
-  // Coherence checks for each returned version.
-  for (const version of vers) {
-    const tested = util.inspect(version);
-    const parts = version.num.split('.');
-    const expectedLength = parts[0] === '0' ? 3 : 2;
-    assert.strictEqual(parts.length, expectedLength,
-                       `'num' from ${tested} should be '<major>.x'.`);
-    assert.strictEqual(parts[parts.length - 1], 'x',
-                       `'num' from ${tested} doesn't end in '.x'.`);
-    const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
-    const hasLtsProperty = version.hasOwnProperty('lts');
-    if (hasLtsProperty) {
-      // Odd-numbered versions of Node.js are never LTS.
-      assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
-      assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
-    }
-  }
+tmpdir.refresh();
+const versionsFile = path.join(tmpdir.path, 'versions.json');
+debuglog(versionsFile);
+const opts = { cwd: tmpdir.path, encoding: 'utf8' };
+const cp = spawnSync(process.execPath, [ versionsTool, versionsFile ], opts);
+debuglog(cp.stderr);
+debuglog(cp.stdout);
+assert.strictEqual(cp.stdout, '');
+assert.strictEqual(cp.signal, null);
+assert.strictEqual(cp.status, 0);
+const versions = JSON.parse(fs.readFileSync(versionsFile));
+debuglog(versions);
 
-  // Check that the minimum number of versions were returned.
-  // Later versions are allowed, but not checked for here (they were checked
-  // above).
-  // Also check for the previous semver major -- From master this will be the
-  // most recent major release.
-  const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
-  const prevMajorString = `${thisMajor - 1}.x`;
-  if (!expected.includes(prevMajorString)) {
-    expected.unshift(prevMajorString);
-  }
-  for (const version of expected) {
-    assert.ok(vers.find((x) => x.num === version),
-              `Did not find entry for '${version}' in ${util.inspect(vers)}`);
+// Coherence checks for each returned version.
+for (const version of versions) {
+  const tested = util.inspect(version);
+  const parts = version.num.split('.');
+  const expectedLength = parts[0] === '0' ? 3 : 2;
+  assert.strictEqual(parts.length, expectedLength,
+                     `'num' from ${tested} should be '<major>.x'.`);
+  assert.strictEqual(parts[parts.length - 1], 'x',
+                     `'num' from ${tested} doesn't end in '.x'.`);
+  const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
+  const hasLtsProperty = version.hasOwnProperty('lts');
+  if (hasLtsProperty) {
+    // Odd-numbered versions of Node.js are never LTS.
+    assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
+    assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
   }
 }
-test();
+
+// Check that the minimum number of versions were returned.
+// Later versions are allowed, but not checked for here (they were checked
+// above).
+// Also check for the previous semver major -- From master this will be the
+// most recent major release.
+const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
+const prevMajorString = `${thisMajor - 1}.x`;
+if (!expected.includes(prevMajorString)) {
+  expected.unshift(prevMajorString);
+}
+for (const version of expected) {
+  assert.ok(versions.find((x) => x.num === version),
+            `Did not find entry for '${version}' in ${util.inspect(versions)}`);
+}