fix: remove ora and use listr instead, make tests run on node (#949)

Removes esbuild-register as it tries to load modules via cjs paths which break with esm. Ts-node doesn't seem to work either so for now we have to build before running tests.
ipfs · Apr 7, 2022 · dd5583a · dd5583a
1 parent 5d448e7
commit dd5583a
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 97 deletions.
diff --git a/package.json b/package.json
@@ -56,8 +56,9 @@
     }
   },
   "files": [
+    "dist",
     "src",
-    "dist/src",
+    "utils",
     "!dist/test",
     "!**/*.tsbuildinfo"
   ],
@@ -195,7 +196,6 @@
     "electron-mocha-main": "^11.0.3",
     "env-paths": "^3.0.0",
     "esbuild": "^0.14.31",
-    "esbuild-register": "^3.3.2",
     "eslint": "^8.12.0",
     "eslint-config-ipfs": "^2.1.0",
     "eslint-plugin-etc": "^2.0.2",
@@ -215,7 +215,6 @@
     "mocha": "^9.0.2",
     "npm-package-json-lint": "^6.3.0",
     "nyc": "^15.1.0",
-    "ora": "^6.1.0",
     "p-map": "^5.3.0",
     "pascalcase": "^2.0.0",
     "path": "^0.12.7",

diff --git a/src/cmds/dependency-check.js b/src/cmds/dependency-check.js
@@ -1,6 +1,5 @@
 /* eslint-disable no-console */
 
-import ora from 'ora'
 import { loadUserConfig } from '../config/user.js'
 import depCheck from '../dependency-check.js'
 
@@ -51,14 +50,6 @@ export default {
    * @param {any} argv
    */
   async handler (argv) {
-    const spinner = ora('Checking dependencies').start()
-
-    try {
-      await depCheck(argv)
-      spinner.succeed()
-    } catch (err) {
-      spinner.fail()
-      throw err
-    }
+    await depCheck.run(argv)
   }
 }
diff --git a/src/config/user.js b/src/config/user.js
@@ -99,6 +99,7 @@ const defaults = {
       'src/**/*.cjs',
       'test/**/*.js',
       'test/**/*.cjs',
+      'dist/**/*.js',
       'benchmarks/**/*.js',
       'benchmarks/**/*.cjs',
       'utils/**/*.js',

diff --git a/src/dependency-check.js b/src/dependency-check.js
@@ -3,11 +3,12 @@ import { execa } from 'execa'
 import merge from 'merge-options'
 import { pkg } from './utils.js'
 import { fileURLToPath } from 'url'
+import Listr from 'listr'
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 
 /**
- * @typedef {import("execa").Options} ExecaOptions
+ * @typedef {import("listr").ListrTaskWrapper} Task
  * @typedef {import("./types").GlobalOptions} GlobalOptions
  * @typedef {import("./types").DependencyCheckOptions} DependencyCheckOptions
  */
@@ -18,43 +19,50 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url))
  */
 const isDefaultInput = (arr1, arr2) =>
   JSON.stringify(arr1) === JSON.stringify(arr2)
-/**
- * Check dependencies
- *
- * @param {GlobalOptions & DependencyCheckOptions} argv - Command line arguments passed to the process.
- * @param {ExecaOptions} [execaOptions] - execa options.
- */
-export default (argv, execaOptions) => {
-  const forwardOptions = argv['--'] ? argv['--'] : []
-  const input =
-        argv.productionOnly &&
-        isDefaultInput(argv.fileConfig.dependencyCheck.input, argv.input)
-          ? argv.fileConfig.dependencyCheck.productionInput
-          : argv.input
-  const noDev = argv.productionOnly ? ['--no-dev'] : []
-  const ignore = argv.ignore
-    .concat(argv.fileConfig.dependencyCheck.ignore)
-    .reduce((acc, i) => acc.concat('-i', i), /** @type {string[]} */ ([]))
-
-  const args = [...input, '--missing', ...noDev, ...ignore]
-
-  if (pkg.type === 'module') {
-    // use detective-es6 for js, regular detective for cjs
-    args.push(
-      '--extensions', 'cjs:detective-cjs',
-      '--extensions', 'js:detective-es6'
-    )
-  }
-
-  return execa(
-    'dependency-check',
-    [...args, ...forwardOptions],
-    merge(
-      {
-        localDir: path.join(__dirname, '..'),
-        preferLocal: true
-      },
-      execaOptions
-    )
-  )
-}
+
+const tasks = new Listr(
+  [
+    {
+      title: 'eslint',
+      /**
+       * @param {GlobalOptions & DependencyCheckOptions} ctx
+       * @param {Task} task
+       */
+      task: async (ctx, task) => {
+        const forwardOptions = ctx['--'] ? ctx['--'] : []
+        const input =
+            ctx.productionOnly &&
+              isDefaultInput(ctx.fileConfig.dependencyCheck.input, ctx.input)
+              ? ctx.fileConfig.dependencyCheck.productionInput
+              : ctx.input
+        const noDev = ctx.productionOnly ? ['--no-dev'] : []
+        const ignore = ctx.ignore
+          .concat(ctx.fileConfig.dependencyCheck.ignore)
+          .reduce((acc, i) => acc.concat('-i', i), /** @type {string[]} */ ([]))
+
+        const args = [...input, '--missing', ...noDev, ...ignore]
+
+        if (pkg.type === 'module') {
+          // use detective-es6 for js, regular detective for cjs
+          args.push(
+            '--extensions', 'cjs:detective-cjs',
+            '--extensions', 'js:detective-es6'
+          )
+        }
+
+        await execa(
+          'dependency-check',
+          [...args, ...forwardOptions],
+          merge(
+            {
+              localDir: path.join(__dirname, '..'),
+              preferLocal: true
+            }
+          )
+        )
+      }
+    }
+  ]
+)
+
+export default tasks
diff --git a/src/lint.js b/src/lint.js
@@ -17,7 +17,6 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url))
  * @typedef {import("./types").LintOptions} LintOptions
  * @typedef {import("listr").ListrTaskWrapper} Task
  * @typedef {import("./types").TSOptions} TSOptions
- *
  */
 
 const tasks = new Listr(

diff --git a/src/test/electron.js b/src/test/electron.js
@@ -1,11 +1,9 @@
 import path from 'path'
 import { execa } from 'execa'
-import { getElectron, isTypescript } from '../utils.js'
+import { getElectron } from '../utils.js'
 import merge from 'merge-options'
 import { fileURLToPath } from 'url'
-import { createRequire } from 'module'
 
-const require = createRequire(import.meta.url)
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 
 /**
@@ -22,13 +20,17 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url))
 export default async (argv, execaOptions) => {
   const forwardOptions = argv['--'] ? argv['--'] : []
   const watch = argv.watch ? ['--watch'] : []
-  const files = argv.files.length > 0 ? [...argv.files] : ['test/**/*.spec.{js,ts,mjs,cjs}']
+  const files = argv.files.length > 0
+    ? [...argv.files]
+    : [
+        'test/**/*.spec.{js,ts,mjs,cjs}',
+        'dist/test/**/*.spec.{js,cjs,mjs}'
+      ]
   const grep = argv.grep ? ['--grep', argv.grep] : []
   const progress = argv.progress ? ['--reporter=progress'] : []
   const bail = argv.bail ? ['--bail'] : []
   const timeout = argv.timeout ? [`--timeout=${argv.timeout}`] : []
   const renderer = argv.runner === 'electron-renderer' ? ['--renderer'] : []
-  const ts = isTypescript ? ['--require', require.resolve('esbuild-register')] : []
 
   // before hook
   const before = await argv.fileConfig.test.before(argv)
@@ -43,7 +45,6 @@ export default async (argv, execaOptions) => {
       ...progress,
       ...bail,
       ...timeout,
-      ...ts,
       '--colors',
       '--full-trace',
       ...renderer,

diff --git a/src/test/node.js b/src/test/node.js
@@ -2,11 +2,8 @@ import { execa } from 'execa'
 import path from 'path'
 import tempy from 'tempy'
 import merge from 'merge-options'
-import { isTypescript } from '../utils.js'
 import { fileURLToPath } from 'url'
-import { createRequire } from 'module'
 
-const require = createRequire(import.meta.url)
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 
 /**
@@ -35,8 +32,10 @@ export default async function testNode (argv, execaOptions) {
   const files = argv.files.length > 0
     ? argv.files
     : [
-        'test/node.{js,ts,cjs,mjs}',
-        'test/**/*.spec.{js,ts,cjs,mjs}'
+        'test/node.{js,cjs,mjs}',
+        'test/**/*.spec.{js,cjs,mjs}',
+        'dist/test/node.{js,cjs,mjs}',
+        'dist/test/**/*.spec.{js,cjs,mjs}'
       ]
 
   const args = [
@@ -60,10 +59,6 @@ export default async function testNode (argv, execaOptions) {
     args.push('--bail')
   }
 
-  if (isTypescript) {
-    args.push(...['--require', require.resolve('esbuild-register')])
-  }
-
   if (argv['--']) {
     args.push(...argv['--'])
   }

diff --git a/src/utils.js b/src/utils.js
@@ -7,7 +7,6 @@
 
 import { constants, createBrotliCompress, createGzip } from 'zlib'
 import os from 'os'
-import ora from 'ora'
 import extract from 'extract-zip'
 import stripComments from 'strip-json-comments'
 import stripBom from 'strip-bom'
@@ -20,6 +19,7 @@ import { execa } from 'execa'
 import envPaths from 'env-paths'
 import lockfile from 'proper-lockfile'
 import { fileURLToPath } from 'url'
+import Listr from 'listr'
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 const EnvPaths = envPaths('aegir', { suffix: '' })
@@ -151,16 +151,31 @@ export const getElectron = async () => {
     lockfilePath
   })
 
-  const version = pkg.devDependencies.electron.slice(1)
-  const spinner = ora(`Downloading electron: ${version}`).start()
-  const zipPath = await download(version)
-  const electronPath = path.join(path.dirname(zipPath), getPlatformPath())
-  if (!fs.existsSync(electronPath)) {
-    spinner.text = 'Extracting electron to system cache'
-    await extract(zipPath, { dir: path.dirname(zipPath) })
+  const aegirManifest = await fs.readJSON(path.join(__dirname, '../package.json'))
+  const version = pkg.devDependencies?.electron?.slice(1) ?? pkg.dependencies?.electron?.slice(1) ?? aegirManifest.devDependencies.electron.slice(1)
+  let electronPath = ''
+  let zipPath = ''
+
+  const tasks = new Listr([{
+    title: `Downloading electron: ${version}`,
+    task: async () => {
+      zipPath = await download(version)
+      electronPath = path.join(path.dirname(zipPath), getPlatformPath())
+    }
+  }, {
+    title: 'Extracting electron to system cache',
+    enabled: () => !fs.existsSync(electronPath),
+    task: async () => {
+      await extract(zipPath, { dir: path.dirname(zipPath) })
+    }
+  }])
+
+  try {
+    await tasks.run()
+  } finally {
+    await releaseLock()
   }
-  spinner.stop()
-  await releaseLock()
+
   return electronPath
 }
 

diff --git a/utils/chai.js b/utils/chai.js
@@ -14,24 +14,13 @@ chai.use(chaiString)
 
 export const expect = chai.expect
 export const assert = chai.assert
-export { chai }
-/*
-module.exports = {
-  expect,
-  assert,
+export {
   chai,
 
   // this is to ensure that we import the chai types in the generated .d.ts file
-  _: {
-    chaiAsPromised,
-    chaiParentheses,
-    chaiSubset,
-    chaiBites,
-    chaiString
-  }
+  chaiAsPromised,
+  chaiParentheses,
+  chaiSubset,
+  chaiBites,
+  chaiString
 }
-
-// we don't actually want to export these things so remove the property
-// @ts-ignore - the operand should be optional
-delete module.exports._
-*/