tests: switch test runner to mocha (#3336)

- test: install deps with --omit=optional (go faster)
- tests: lots of cleanups for es6 & brevity

.github/workflows/ci.yml

@@ -10,7 +10,7 @@ jobs:
     uses: haraka/.github/.github/workflows/lint.yml@master
 
   test:
-    needs: [ lint, get-lts ]
+    needs: [ get-lts ]
     runs-on: ${{ matrix.os }}
     services:
       redis:
@@ -28,9 +28,7 @@ jobs:
       name: Node.js ${{ matrix.node-version }} on ${{ matrix.os }}
       with:
         node-version: ${{ matrix.node-version }}
-    - name: Install bsdtar
-      run: sudo apt-get update; sudo apt-get install -y libarchive-tools
-    - run: npm install
+    - run: npm install --omit=optional
     - run: npm run test
 
   # TODO: replace the above with this, after plugin/attachment is split

.github/workflows/coverage.yml

@@ -9,8 +9,8 @@ jobs:
   coverage:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v3
-    - uses: actions/setup-node@v3
+    - uses: actions/checkout@v4
+    - uses: actions/setup-node@v4
       name: Node.js
 
     - name: install libarchive-tools

Changes.md

@@ -47,6 +47,7 @@
 - remove last vestiges of header_hide_version (long ago renamed)
 - server.js: use the local logger methods
 - get Haraka version from utils.getVersion (which includes git id if running from repo)
+- test: convert test runner to mocha
 
 #### Fixed
 

bin/haraka

@@ -415,7 +415,7 @@ else if (parsed.test) {
             port: 1234,
         },
         destroy () {},
-        on (event, done) {},
+        on (event) {},
         end () {
             process.exit();
         },

outbound/queue.js

@@ -75,7 +75,6 @@ exports.stat_queue = cb => {
 exports.load_queue = pid => {
     // Initialise and load queue
     // This function is called first when not running under cluster,
-    // so we create the queue directory if it doesn't already exist.
     exports.ensure_queue_dir();
     exports.delete_dot_files();
 
@@ -201,7 +200,6 @@ exports.load_queue_files = (pid, input_files, iteratee, callback = function () {
 }
 
 exports.stats = () => {
-
     return {
         queue_dir,
         queue_count,
@@ -226,7 +224,7 @@ exports._list_file = (file, cb) => {
                 // we read everything
                 const todo_struct = JSON.parse(todo);
                 todo_struct.rcpt_to = todo_struct.rcpt_to.map(a => new Address (a));
-                todo_struct.mail_from = new Address (todo_struct.mail_from);
+                todo_struct.mail_from = new Address(todo_struct.mail_from);
                 todo_struct.file = file;
                 todo_struct.full_path = path.join(queue_dir, file);
                 const parts = _qfile.parts(file);
@@ -247,12 +245,12 @@ exports.flush_queue = (domain, pid) => {
     if (domain) {
         exports.list_queue((err, qlist) => {
             if (err) return logger.error(exports, `Failed to load queue: ${err}`);
-            qlist.forEach(todo => {
+            for (const todo of qlist) {
                 if (todo.domain.toLowerCase() != domain.toLowerCase()) return;
                 if (pid && todo.pid != pid) return;
                 // console.log("requeue: ", todo);
                 delivery_queue.push(new HMailItem(todo.file, todo.full_path));
-            });
+            }
         })
     }
     else {
@@ -266,7 +264,6 @@ exports.load_pid_queue = pid => {
 }
 
 exports.ensure_queue_dir = () => {
-    // No reason to do this asynchronously
     // this code is only run at start-up.
     if (fs.existsSync(queue_dir)) return;
 
@@ -316,8 +313,7 @@ exports._add_hmail = hmail => {
 exports.scan_queue_pids = cb => {
     const self = exports;
 
-    // Under cluster, this is called first by the master so
-    // we create the queue directory if it doesn't exist.
+    // Under cluster, this is called first by the master
     self.ensure_queue_dir();
     self.delete_dot_files();
 

outbound/timer_queue.js

@@ -21,6 +21,7 @@ class TimerQueue {
     constructor (interval = 1000) {
         this.queue = [];
         this.interval_timer = setInterval(() => { this.fire(); }, interval);
+        this.interval_timer.unref() // allow server to exit
     }
 
     add (id, ms, cb) {

package.json

@@ -24,26 +24,27 @@
     "address-rfc2822": "^2.2.1",
     "async": "^3.2.5",
     "daemon": "~1.1.0",
-    "ipaddr.js": "~2.2.0",
-    "node-gyp": "^10.1.0",
-    "nopt": "~7.2.0",
-    "npid": "~0.4.0",
-    "semver": "~7.6.0",
-    "sprintf-js": "~1.1.3",
-    "haraka-config": "^1.1.0",
+    "haraka-config": "^1.2.4",
     "haraka-constants": "^1.0.6",
     "haraka-dsn": "^1.0.5",
-    "haraka-email-message": "^1.2.2",
+    "haraka-email-message": "^1.2.3",
     "haraka-message-stream": "^1.2.1",
     "haraka-net-utils": "^1.6.0",
     "haraka-notes": "^1.0.7",
     "haraka-plugin-redis": "^2.0.7",
+    "haraka-plugin-spf": "1.2.5",
     "haraka-results": "^2.2.4",
     "haraka-tld": "^1.2.1",
     "haraka-utils": "^1.1.3",
+    "ipaddr.js": "~2.2.0",
+    "node-gyp": "^10.1.0",
+    "nopt": "~7.2.0",
+    "npid": "~0.4.0",
     "openssl-wrapper": "^0.3.4",
     "redis": "~4.6.13",
-    "sockaddr": "^1.0.1"
+    "semver": "^7.6.0",
+    "sockaddr": "^1.0.1",
+    "sprintf-js": "~1.1.3"
   },
   "optionalDependencies": {
     "haraka-plugin-access": "^1.1.6",
@@ -56,29 +57,28 @@
     "haraka-plugin-dns-list": "^1.2.0",
     "haraka-plugin-elasticsearch": "^8.0.2",
     "haraka-plugin-fcrdns": "^1.1.0",
-    "haraka-plugin-graph": "^1.0.5",
     "haraka-plugin-geoip": "^1.1.0",
+    "haraka-plugin-graph": "^1.0.5",
     "haraka-plugin-headers": "^1.0.4",
     "haraka-plugin-karma": "^2.1.5",
+    "haraka-plugin-known-senders": "^1.1.0",
     "haraka-plugin-limit": "^1.2.3",
     "haraka-plugin-p0f": "^1.0.9",
     "haraka-plugin-qmail-deliverable": "^1.2.3",
-    "haraka-plugin-known-senders": "^1.1.0",
     "haraka-plugin-rcpt-ldap": "^1.1.0",
     "haraka-plugin-recipient-routes": "^1.2.0",
     "haraka-plugin-rspamd": "^1.3.1",
-    "haraka-plugin-spf": "1.2.5",
     "haraka-plugin-syslog": "^1.0.6",
     "haraka-plugin-uribl": "^1.0.8",
     "haraka-plugin-watch": "^2.0.4",
     "ocsp": "~1.2.0",
     "tmp": "~0.2.3"
   },
   "devDependencies": {
-    "nodeunit-x": "^0.16.0",
+    "@haraka/eslint-config": "^1.1.5",
     "haraka-test-fixtures": "^1.3.7",
+    "mocha": "^10.4.0",
     "mock-require": "^3.0.3",
-    "@haraka/eslint-config": "^1.1.5",
     "nodemailer": "^6.9.13"
   },
   "bugs": {
@@ -90,9 +90,9 @@
     "haraka_grep": "./bin/haraka_grep"
   },
   "scripts": {
-    "test": "node --use_strict run_tests",
+    "test": "npx mocha --exit --timeout=3000 tests tests/outbound tests/plugins/auth tests/plugins/queue tests/plugins",
     "lint": "npx eslint@^8 *.js outbound plugins plugins/*/*.js tests tests/*/*.js tests/*/*/*.js bin/haraka",
-    "lintfix": "npx eslint@^8 --fix *.js outbound plugins plugins/*/*.js tests tests/*/*.js tests/*/*/*.js bin/haraka",
+    "lint:fix": "npx eslint@^8 --fix *.js outbound plugins plugins/*/*.js tests tests/*/*.js tests/*/*/*.js bin/haraka",
     "versions": "npx dependency-version-checker check",
     "versions:fix": "npx dependency-version-checker update && npm run prettier:fix"
   }

run_tests

@@ -1,32 +1,5 @@
-#!/usr/bin/env node
+#!/bin/sh
 
-'use strict';
+TESTS=${1:-"tests tests/outbound tests/plugins/auth tests/plugins/queue tests/plugins"}
 
-let reporter;
-try {
-    reporter = require('nodeunit-x').reporters.default;
-}
-catch (e) {
-    console.log(`Error: ${e.message}
-
-Cannot find nodeunit module. Please run the following:
-
-    npm install\n`);
-
-    process.exit();
-}
-
-process.chdir(__dirname);
-
-let tests = [
-    'tests', 'tests/outbound', 'tests/plugins',
-    'tests/plugins/auth', 'tests/plugins/queue'
-];
-
-if (process.argv[2]) {
-    console.log("Running tests: ", process.argv.slice(2));
-    tests = process.argv.slice(2);
-}
-reporter.run(tests, undefined, function (err) {
-    process.exit(((err) ? 1 : 0));
-});
+npx mocha --exit $TESTS

server.js

@@ -220,7 +220,7 @@ Server._graceful = shutdown => {
         if (shutdown) {
             Server.loginfo("Workers closed. Shutting down master process subsystems");
             for (const module of ['outbound', 'cfreader', 'plugins']) {
-                process.emit('message', {event: `${module  }.shutdown`});
+                process.emit('message', {event: `${module}.shutdown`});
             }
             const t2 = setTimeout(shutdown, Server.cfg.main.force_shutdown_timeout * 1000);
             return t2.unref();

tests/config/helo.checks.ini

@@ -14,7 +14,7 @@ forward_dns=true
 rdns_match=true
 ; host_mismatch:  hostname differs between EHLO invocations
 host_mismatch=true
-proto_mismatch: host sent EHLO but then tries to sent HELO or vice-versa
+; proto_mismatch: host sent EHLO but then tries to sent HELO or vice-versa
 proto_mismatch=true
 
 [reject]