fix: remove custom test reporter

Author: lukekarrys

.github/PULL_REQUEST_TEMPLATE.md

@@ -7,7 +7,7 @@ Contributor guide: https://github.com/nodejs/node/blob/main/CONTRIBUTING.md
 ##### Checklist
 <!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->
 
-- [ ] `npm install && npm test` passes
+- [ ] `npm install && npm run lint && npm test` passes
 - [ ] tests are included <!-- Bug fixes and new features should include tests -->
 - [ ] documentation is changed or added
 - [ ] commit message follows [commit guidelines](https://github.com/googleapis/release-please#how-should-i-write-my-commits)

.github/workflows/tests.yml

@@ -18,11 +18,24 @@ jobs:
     - uses: actions/checkout@v4
     - run: pip install --user ruff
     - run: ruff --output-format=github --select="E,F,PLC,PLE,UP,W,YTT" --ignore="E721,PLC1901,S101,UP031" --target-version=py38 .
+  Lint_JS:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout Repository
+      uses: actions/checkout@v4
+    - name: Use Node.js 20.x
+      uses: actions/setup-node@v3
+      with:
+        node-version: 20.x
+    - name: Install Dependencies
+      run: npm install --no-progress
+    - name: Lint
+      run: npm run lint
   Engines:
     runs-on: ubuntu-latest
     steps:
     - name: Checkout Repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
     - name: Use Node.js 20.x
       uses: actions/setup-node@v3
       with:
@@ -77,7 +90,11 @@ jobs:
         if: runner.os != 'Windows'
         shell: bash
         run: npm test --python="${pythonLocation}/python"
+        env:
+          FAST_TEST: ${{ (matrix.node == '20.x' && matrix.python == '3.12') && '0' || '1' }}
       - name: Run tests (Windows)
         if: runner.os == 'Windows'
         shell: pwsh
         run: npm run test --python="${env:pythonLocation}\\python.exe"
+        env:
+          FAST_TEST: ${{ (matrix.node == '20.x' && matrix.python == '3.12') && '0' || '1' }}

bin/node-gyp.js

@@ -32,9 +32,9 @@ if (prog.devDir) {
 
 if (prog.todo.length === 0) {
   if (~process.argv.indexOf('-v') || ~process.argv.indexOf('--version')) {
-    console.log('v%s', prog.version)
+    log.stdout('v%s', prog.version)
   } else {
-    console.log('%s', prog.usage())
+    log.stdout('%s', prog.usage())
   }
   process.exit(0)
 }
@@ -82,12 +82,12 @@ async function run () {
 
     if (command.name === 'list') {
       if (args.length) {
-        args.forEach((version) => console.log(version))
+        args.forEach((version) => log.stdout(version))
       } else {
-        console.log('No node development files installed. Use `node-gyp install` to install a version.')
+        log.stdout('No node development files installed. Use `node-gyp install` to install a version.')
       }
     } else if (args.length >= 1) {
-      console.log(...args.slice(1))
+      log.stdout(...args.slice(1))
     }
 
     // now run the next command in the queue

lib/install.js

@@ -14,7 +14,7 @@ const processRelease = require('./process-release')
 const win = process.platform === 'win32'
 
 async function install (gyp, argv) {
-  console.log()
+  log.stdout()
   const release = processRelease(argv, gyp, process.version, process.release)
   // Detecting target_arch based on logic from create-cnfig-gyp.js. Used on Windows only.
   const arch = win ? (gyp.opts.target_arch || gyp.opts.arch || process.arch || 'ia32') : ''

lib/log.js

@@ -73,11 +73,11 @@ class Logger {
     style: { fg: 'red', bg: 'black' }
   }]
 
-  constructor () {
+  constructor (stream) {
     process.on('log', (...args) => this.#onLog(...args))
     this.#levels = new Map(this.#levels.map((level, index) => [level.id, { ...level, index }]))
     this.level = 'info'
-    this.stream = process.stderr
+    this.stream = stream
     procLog.pause()
   }
 
@@ -158,8 +158,12 @@ class Logger {
   }
 }
 
+// used to suppress logs in tests
+const NULL_LOGGER = !!process.env.NODE_GYP_NULL_LOGGER
+
 module.exports = {
-  logger: new Logger(),
+  logger: new Logger(NULL_LOGGER ? null : process.stderr),
+  stdout: NULL_LOGGER ? () => {} : (...args) => console.log(...args),
   withPrefix,
   ...procLog
 }

package.json

@@ -45,6 +45,6 @@
   },
   "scripts": {
     "lint": "standard \"*/*.js\" \"test/**/*.js\" \".github/**/*.js\"",
-    "test": "npm run lint && mocha --timeout 15000 --reporter=test/reporter.js test/test-download.js test/test-*"
+    "test": "NODE_GYP_NULL_LOGGER=true mocha --timeout 15000 test/test-download.js test/test-*"
   }
 }

test/common.js

@@ -1,4 +1,5 @@
 const envPaths = require('env-paths')
+const semver = require('semver')
 
 module.exports.devDir = () => envPaths('node-gyp', { suffix: '' }).cache
 
@@ -15,3 +16,7 @@ module.exports.poison = (object, property) => {
   }
   Object.defineProperty(object, property, descriptor)
 }
+
+module.exports.skip = process.env.FAST_TEST === '1' ||
+  process.release.name !== 'node' ||
+  semver.prerelease(process.version) !== null

test/reporter.js

@@ -4,55 +4,56 @@ const { describe, it } = require('mocha')
 const assert = require('assert')
 const path = require('path')
 const fs = require('graceful-fs')
-const { execFileSync, execFile } = require('child_process')
 const os = require('os')
+const cp = require('child_process')
+const util = require('../lib/util')
+
 const addonPath = path.resolve(__dirname, 'node_modules', 'hello_world')
 const nodeGyp = path.resolve(__dirname, '..', 'bin', 'node-gyp.js')
 
-function runHello (hostProcess) {
-  if (!hostProcess) {
-    hostProcess = process.execPath
-  }
+const execFileSync = (...args) => cp.execFileSync(...args).toString().trim()
+
+const execFile = async (cmd) => {
+  const [err,, stderr] = await util.execFile(process.execPath, cmd, {
+    env: { ...process.env, NODE_GYP_NULL_LOGGER: undefined },
+    encoding: 'utf-8'
+  })
+  return [err, stderr.toString().trim().split(/\r?\n/)]
+}
+
+function runHello (hostProcess = process.execPath) {
   const testCode = "console.log(require('hello_world').hello())"
-  return execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname }).toString()
+  return execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname })
 }
 
 function getEncoding () {
   const code = 'import locale;print(locale.getdefaultlocale()[1])'
-  return execFileSync('python', ['-c', code]).toString().trim()
+  return execFileSync('python', ['-c', code])
 }
 
 function checkCharmapValid () {
-  let data
   try {
-    data = execFileSync('python', ['fixtures/test-charmap.py'],
-      { cwd: __dirname })
-  } catch (err) {
+    const data = execFileSync('python', ['fixtures/test-charmap.py'], { cwd: __dirname })
+    return data.split('\n').pop() === 'True'
+  } catch {
     return false
   }
-  const lines = data.toString().trim().split('\n')
-  return lines.pop() === 'True'
 }
 
 describe('addon', function () {
   this.timeout(300000)
 
-  it('build simple addon', function (done) {
+  it('build simple addon', async function () {
     // Set the loglevel otherwise the output disappears when run via 'npm test'
     const cmd = [nodeGyp, 'rebuild', '-C', addonPath, '--loglevel=verbose']
-    const proc = execFile(process.execPath, cmd, function (err, stdout, stderr) {
-      const logLines = stderr.toString().trim().split(/\r?\n/)
-      const lastLine = logLines[logLines.length - 1]
-      assert.strictEqual(err, null)
-      assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
-      assert.strictEqual(runHello().trim(), 'world')
-      done()
-    })
-    proc.stdout.setEncoding('utf-8')
-    proc.stderr.setEncoding('utf-8')
+    const [err, logLines] = await execFile(cmd)
+    const lastLine = logLines[logLines.length - 1]
+    assert.strictEqual(err, null)
+    assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
+    assert.strictEqual(runHello(), 'world')
   })
 
-  it('build simple addon in path with non-ascii characters', function (done) {
+  it('build simple addon in path with non-ascii characters', async function () {
     if (!checkCharmapValid()) {
       return this.skip('python console app can\'t encode non-ascii character.')
     }
@@ -105,46 +106,30 @@ describe('addon', function () {
       '--loglevel=verbose',
       '-nodedir=' + testNodeDir
     ]
-    const proc = execFile(process.execPath, cmd, function (err, stdout, stderr) {
-      try {
-        fs.unlink(testNodeDir)
-      } catch (err) {
-        assert.fail(err)
-      }
-
-      const logLines = stderr.toString().trim().split(/\r?\n/)
-      const lastLine = logLines[logLines.length - 1]
-      assert.strictEqual(err, null)
-      assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
-      assert.strictEqual(runHello().trim(), 'world')
-      done()
-    })
-    proc.stdout.setEncoding('utf-8')
-    proc.stderr.setEncoding('utf-8')
-  })
-
-  it('addon works with renamed host executable', function (done) {
-    // No `fs.copyFileSync` before node8.
-    if (process.version.substr(1).split('.')[0] < 8) {
-      return this.skip('skipping test for old node version')
+    const [err, logLines] = await execFile(cmd)
+    try {
+      fs.unlink(testNodeDir)
+    } catch (err) {
+      assert.fail(err)
     }
+    const lastLine = logLines[logLines.length - 1]
+    assert.strictEqual(err, null)
+    assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
+    assert.strictEqual(runHello(), 'world')
+  })
 
+  it('addon works with renamed host executable', async function () {
     this.timeout(300000)
 
     const notNodePath = path.join(os.tmpdir(), 'notnode' + path.extname(process.execPath))
     fs.copyFileSync(process.execPath, notNodePath)
 
     const cmd = [nodeGyp, 'rebuild', '-C', addonPath, '--loglevel=verbose']
-    const proc = execFile(process.execPath, cmd, function (err, stdout, stderr) {
-      const logLines = stderr.toString().trim().split(/\r?\n/)
-      const lastLine = logLines[logLines.length - 1]
-      assert.strictEqual(err, null)
-      assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
-      assert.strictEqual(runHello(notNodePath).trim(), 'world')
-      fs.unlinkSync(notNodePath)
-      done()
-    })
-    proc.stdout.setEncoding('utf-8')
-    proc.stderr.setEncoding('utf-8')
+    const [err, logLines] = await execFile(cmd)
+    const lastLine = logLines[logLines.length - 1]
+    assert.strictEqual(err, null)
+    assert.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
+    assert.strictEqual(runHello(notNodePath), 'world')
+    fs.unlinkSync(notNodePath)
   })
 })

test/test-addon.js

@@ -5,8 +5,8 @@ const assert = require('assert')
 const path = require('path')
 const devDir = require('./common').devDir()
 const gyp = require('../lib/node-gyp')
-const log = require('../lib/log')
 const requireInject = require('require-inject')
+
 const configure = requireInject('../lib/configure', {
   'graceful-fs': {
     openSync: () => 0,
@@ -19,8 +19,6 @@ const configure = requireInject('../lib/configure', {
   }
 })
 
-log.logger.stream = null
-
 const EXPECTED_PYPATH = path.join(__dirname, '..', 'gyp', 'pylib')
 const SEPARATOR = process.platform === 'win32' ? ';' : ':'
 const SPAWN_RESULT = cb => ({ on: function () { cb() } })