Repository: finos/git-proxy

Issue:
Fix failing plugin tests - `ts-mocha` compatibility issue
These three tests from `test/plugin/plugin.test.js` started failing since we upgraded all our files from CommonJS + JS to ESM + TS. This is due to `ts-mocha` tripping up with the way our `PluginLoader` works. Note that **this issue is only reproducible in Node 18 to 21**, the problem is gone in Node 22 due to interoperability improvements.

<details>
<summary>Problematic tests</summary

```js
  it('should load plugins that are the default export (module.exports = pluginObj)', async function () {
    const loader = new PluginLoader([join(testPackagePath, 'default-export.js')]);
    await loader.load();
    expect(loader.pushPlugins.length).to.equal(1);
    expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true;
    expect(loader.pushPlugins[0]).to.be.an.instanceOf(PushActionPlugin);
  }).timeout(10000);

  it('should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })', async function () {
    const loader = new PluginLoader([join(testPackagePath, 'multiple-export.js')]);
    await loader.load();
    expect(loader.pushPlugins.length).to.equal(1);
    expect(loader.pullPlugins.length).to.equal(1);
    expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true;
    expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPushActionPlugin'))).to
      .be.true;
    expect(loader.pullPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPullActionPlugin'))).to
      .be.true;
    expect(loader.pushPlugins[0]).to.be.instanceOf(PushActionPlugin);
    expect(loader.pullPlugins[0]).to.be.instanceOf(PullActionPlugin);
  }).timeout(10000);

  it('should load plugins that are subclassed from plugin classes', async function () {
    const loader = new PluginLoader([join(testPackagePath, 'subclass.js')]);
    await loader.load();
    expect(loader.pushPlugins.length).to.equal(1);
    expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p))).to.be.true;
    expect(loader.pushPlugins.every((p) => isCompatiblePlugin(p, 'isGitProxyPushActionPlugin'))).to
      .be.true;
    expect(loader.pushPlugins[0]).to.be.instanceOf(PushActionPlugin);
  }).timeout(10000);
```

</details>

<details>
<summary>Test Execution Output</summary>

```bash
$ nvm use 18
Now using node v18.20.7 (npm v10.8.2)
$ npm run test-plugins

> @finos/git-proxy@1.17.1 test-plugins
> NODE_ENV=test ts-mocha './test/plugin/*.test.js' --exit



  loading plugins from packages
Failed to load plugin: Error [ERR_REQUIRE_ESM]: require() of ES Module /home/juan/Desktop/Juan/Projects/git-proxy/node_modules/load-plugin/index.js from /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts not supported.
Instead change the require of index.js in /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts to a dynamic import() which is available in all CommonJS modules.
Found 0 plugin modules
    1) should load plugins that are the default export (module.exports = pluginObj)
(node:747153) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
(Use `node --trace-warnings ...` to show where the warning was created)
Failed to load plugin: Error [ERR_REQUIRE_ESM]: require() of ES Module /home/juan/Desktop/Juan/Projects/git-proxy/node_modules/load-plugin/index.js from /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts not supported.
Instead change the require of index.js in /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts to a dynamic import() which is available in all CommonJS modules.
Found 0 plugin modules
    2) should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })
Failed to load plugin: Error [ERR_REQUIRE_ESM]: require() of ES Module /home/juan/Desktop/Juan/Projects/git-proxy/node_modules/load-plugin/index.js from /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts not supported.
Instead change the require of index.js in /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts to a dynamic import() which is available in all CommonJS modules.
Found 0 plugin modules
    3) should load plugins that are subclassed from plugin classes
Failed to load plugin: Error [ERR_REQUIRE_ESM]: require() of ES Module /home/juan/Desktop/Juan/Projects/git-proxy/node_modules/load-plugin/index.js from /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts not supported.
Instead change the require of index.js in /home/juan/Desktop/Juan/Projects/git-proxy/src/plugin.ts to a dynamic import() which is available in all CommonJS modules
```

</details>


### Possible solutions:
- Run the CI in Node 22, and enforce Node 22 usage
  - May cause problems with users that cannot upgrade Node for certain reasons
  - By far the easiest fix

### Things I've tried: 
- Fixing the plugin implementation so that the `require` in the culprit code is avoided 
  - Spent a few hours trying to refactor the import (by delaying imports, etc.) without luck
- Fixing the `ts-mocha` execution so that it uses `tsx` loader under the hood
  - Also didn't work for me, however, I can't remember why

### Current solution:
Right now, we're simply skipping the three tests, since they do pass when executed in Node 22 (and they don't throw the `ERR_REQUIRE_ESM` error when executed via `tsx`).

Hints from issue comments:
@kriswest: Node 18 is EOL (last month?), however 20 has a year to go in maintenance unfortunately...

![Image](https://github.com/user-attachments/assets/5781b988-1f7f-4f3b-90fc-750e5d68eee6)

Solution patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 6ea49704..ae41e906 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -19,7 +19,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [18.x]
+        node-version: [20.x]
         mongodb-version: [4.4]
 
     steps:
diff --git a/.github/workflows/experimental-inventory-ci.yml b/.github/workflows/experimental-inventory-ci.yml
index d63b49ad..edd56438 100644
--- a/.github/workflows/experimental-inventory-ci.yml
+++ b/.github/workflows/experimental-inventory-ci.yml
@@ -19,7 +19,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [18.x]
+        node-version: [20.x]
         mongodb-version: [4.4]
 
     steps:
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index b063c9cd..5e4a565d 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -2,8 +2,8 @@ name: Code Cleanliness
 
 on: [pull_request]
 
-env: # environment variables (available in any part of the action)
-  NODE_VERSION: 18
+env:
+  NODE_VERSION: 20
 
 permissions:
   contents: read
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 97af688e..6ba80af9 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -52,7 +52,7 @@
 #       - uses: actions/checkout@8459bc0 # v4
 #       - uses: actions/setup-node@c2ac33f # v4, Setup .npmrc file to publish to npm
 #         with:
-#           node-version: '18.x'
+#           node-version: '20.x'
 #           registry-url: 'https://registry.npmjs.org'
 #       - run: npm ci
 #       - run: npm publish --access=public



Test patch:
diff --git a/test/plugin/plugin.test.js b/test/plugin/plugin.test.js
index cee46699..1180ba9b 100644
--- a/test/plugin/plugin.test.js
+++ b/test/plugin/plugin.test.js
@@ -23,7 +23,7 @@ describe('loading plugins from packages', function () {
     spawnSync('npm', ['install'], { cwd: testPackagePath, timeout: 5000 });
   });
 
-  it.skip('should load plugins that are the default export (module.exports = pluginObj)', async function () {
+  it('should load plugins that are the default export (module.exports = pluginObj)', async function () {
     const loader = new PluginLoader([join(testPackagePath, 'default-export.js')]);
     await loader.load();
     expect(loader.pushPlugins.length).to.equal(1);
@@ -31,7 +31,7 @@ describe('loading plugins from packages', function () {
     expect(loader.pushPlugins[0]).to.be.an.instanceOf(PushActionPlugin);
   }).timeout(10000);
 
-  it.skip('should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })', async function () {
+  it('should load multiple plugins from a module that match the plugin class (module.exports = { pluginFoo, pluginBar })', async function () {
     const loader = new PluginLoader([join(testPackagePath, 'multiple-export.js')]);
     await loader.load();
     expect(loader.pushPlugins.length).to.equal(1);
@@ -45,7 +45,7 @@ describe('loading plugins from packages', function () {
     expect(loader.pullPlugins[0]).to.be.instanceOf(PullActionPlugin);
   }).timeout(10000);
 
-  it.skip('should load plugins that are subclassed from plugin classes', async function () {
+  it('should load plugins that are subclassed from plugin classes', async function () {
     const loader = new PluginLoader([join(testPackagePath, 'subclass.js')]);
     await loader.load();
     expect(loader.pushPlugins.length).to.equal(1);
diff --git a/test/testUserCreation.test.js b/test/testUserCreation.test.js
deleted file mode 100644
index 631be006..00000000
--- a/test/testUserCreation.test.js
+++ /dev/null
@@ -1,88 +0,0 @@
-// Import the dependencies for testing
-const chai = require('chai');
-const bcrypt = require('bcryptjs');
-const chaiHttp = require('chai-http');
-const db = require('../src/db');
-const service = require('../src/service');
-
-chai.use(chaiHttp);
-chai.should();
-const expect = chai.expect;
-const should = chai.should();
-
-describe('user creation', async () => {
-  let app;
-  let cookie;
-
-  const setCookie = function (res) {
-    res.headers['set-cookie'].forEach((x) => {
-      if (x.startsWith('connect')) {
-        const value = x.split(';')[0];
-        cookie = value;
-      }
-    });
-  };
-
-  before(async function () {
-    app = await service.start();
-    await db.deleteUser('login-test-user');
-  });
-
-  it('should be able to login', async function () {
-    const res = await chai.request(app).post('/api/auth/login').send({
-      username: 'admin',
-      password: 'admin',
-    });
-
-    expect(res).to.have.cookie('connect.sid');
-    res.should.have.status(200);
-    setCookie(res);
-  });
-
-  it('should be able to create a new user', async function () {
-    const res = await chai.request(app).post('/api/auth/profile').set('Cookie', `${cookie}`).send({
-      username: 'login-test-user',
-      email: 'paul.timothy.groves@gmail.com',
-      gitAccount: 'test123',
-      admin: true,
-    });
-    res.should.have.status(200);
-  }).skip();
-
-  it('logout', async function () {
-    const res = await chai.request(app).post('/api/auth/logout').set('Cookie', `${cookie}`);
-    res.should.have.status(200);
-  });
-
-  it('login as new user', async function () {
-    // we don't know the users temporary password - so force update a password
-    const user = await db.findUser('login-test-user');
-
-    await bcrypt.hash('test1234', 10, async function (err, hash) {
-      user.password = hash;
-
-      await db.updateUser(user);
-
-      const res = await chai.request(app).post('/api/auth/login').send({
-        username: 'login-test-user',
-        password: 'test1234',
-      });
-
-      expect(res).to.have.cookie('connect.sid');
-      res.should.have.status(200);
-      setCookie(res);
-    });
-  });
-
-  it('should access the profile', async function () {
-    const res = await chai.request(app).get('/api/auth/profile').set('Cookie', `${cookie}`);
-    res.should.have.status(200);
-
-    res.body.username.should.equal('login-test-user');
-    should.not.exist(res.body.password);
-  });
-
-  after(async function () {
-    await service.httpServer.close();
-  });
-});