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,7 +4,7 @@ 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 cp = require('child_process')
 const os = require('os')
 const addonPath = path.resolve(__dirname, 'node_modules', 'hello_world')
 const nodeGyp = path.resolve(__dirname, '..', 'bin', 'node-gyp.js')
@@ -14,18 +14,18 @@ function runHello (hostProcess) {
     hostProcess = process.execPath
   }
   const testCode = "console.log(require('hello_world').hello())"
-  return execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname }).toString()
+  return cp.execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname }).toString()
 }
 
 function getEncoding () {
   const code = 'import locale;print(locale.getdefaultlocale()[1])'
-  return execFileSync('python', ['-c', code]).toString().trim()
+  return cp.execFileSync('python', ['-c', code]).toString().trim()
 }
 
 function checkCharmapValid () {
   let data
   try {
-    data = execFileSync('python', ['fixtures/test-charmap.py'],
+    data = cp.execFileSync('python', ['fixtures/test-charmap.py'],
       { cwd: __dirname })
   } catch (err) {
     return false
@@ -34,13 +34,25 @@ function checkCharmapValid () {
   return lines.pop() === 'True'
 }
 
+const omit = (obj, ...keys) => {
+  const res = {}
+  for (const key of Object.keys(obj)) {
+    if (!keys.includes(key)) {
+      res[key] = obj[key]
+    }
+  }
+  return res
+}
+
 describe('addon', function () {
   this.timeout(300000)
 
+  const execFile = (cmd, fn) => cp.execFile(process.execPath, cmd, { env: omit(process.env, 'NODE_GYP_NULL_LOGGER') }, fn)
+
   it('build simple addon', function (done) {
     // 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 proc = execFile(cmd, function (err, stdout, stderr) {
       const logLines = stderr.toString().trim().split(/\r?\n/)
       const lastLine = logLines[logLines.length - 1]
       assert.strictEqual(err, null)
@@ -105,7 +117,7 @@ describe('addon', function () {
       '--loglevel=verbose',
       '-nodedir=' + testNodeDir
     ]
-    const proc = execFile(process.execPath, cmd, function (err, stdout, stderr) {
+    const proc = execFile(cmd, function (err, stdout, stderr) {
       try {
         fs.unlink(testNodeDir)
       } catch (err) {
@@ -124,18 +136,13 @@ describe('addon', function () {
   })
 
   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')
-    }
-
     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 proc = execFile(cmd, function (err, stdout, stderr) {
       const logLines = stderr.toString().trim().split(/\r?\n/)
       const lastLine = logLines[logLines.length - 1]
       assert.strictEqual(err, null)

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() } })