ci: add test workflow

[andreynering]

No description provided.

[andreynering]

Hey @sounisi5011 and @mheob 👋

I'm sorry to bother you guys, but I think I need some help here.

I wanted to review some PRs today but I lost 1+ hour trying to understand why the test suite was broken for me. I finally decided to add CI and now I see it's not only on my machine. ™️

For some reason the used-pm is unable to be imported on the test suite. I tried lots of things: upgrading my Node, moving it from devDependencies to dependencies, adding a jest.config.js file and tweaking some settings, upgrading jest, but to my frustration nothing worked...

If I return in history to before #3 then it works, because we'd be removing that dependency, but the fact is that it should actually work.

I'd like to know if you guys have any idea on how to fix it. Do the test suite happen to pass on your machines? Specially you @sounisi5011 that worked on #9, do you managed to make it work somehow?

Thanks!

 FAIL  __test__/cli.spec.js
  ● Test suite failed to run

    Cannot find module 'used-pm' from 'common.js'

    However, Jest was able to find:
    	'../common.js'

    You might want to include a file extension in your import, or update your 'moduleFileExtensions', which is currently ['js', 'json', 'jsx', 'ts', 'tsx', 'node'].

    See https://jestjs.io/docs/en/configuration#modulefileextensions-array-string

    However, Jest was able to find:
    	'../src/actions/install.js'

    You might want to include a file extension in your import, or update your 'moduleFileExtensions', which is currently ['js', 'json', 'jsx', 'ts', 'tsx', 'node'].

    See https://jestjs.io/docs/en/configuration#modulefileextensions-array-string

      3 | const { existsSync, readFileSync } = require('fs');
      4 | const mkdirp = require('mkdirp');
    > 5 | const usedPM = require('used-pm');
        |                ^
      6 |
      7 | // Mapping from Node's `process.arch` to Golang's `$GOARCH`
      8 | const ARCH_MAPPING = {

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:259:17)
      at Object.<anonymous> (src/common.js:5:16)

[sounisi5011]

There are two reasons why the test fails:

1. The version of Jest being used is older.

   The package.json of used-pm does not contain the "main" field used by the old Node.js. The Jest used in go-npm is so old that it does not know about the new "exports" field. This is the reason for the "Cannot find module 'used-pm' from 'common.js'" error.

   In fix(unzip): migrate the dependency used to extract zip archives from unzipper to adm-zip #9, I wrote the test command as npm test ./__test__/assets/unzip.spec.js instead of npm test to avoid this. When I wanted to run all tests, I had to manually edit the node_modules/used-pm/package.json file and add "main":"./dist/index.cjs",.

   You can resolve this problem by upgrading Jest to 27.0.0 or later.

2. Some tests have a bug.

   When you update Jest, the "common › getInstallationPath() › should call callback with error if binaries path is not found" test fails. This is because this test code has a bug in it.

   go-npm/__test__/common.spec.js

   Lines 54 to 62 in 357090b

   	it('should call callback with error if binaries path is not found', () => {
   	childProcess.exec.mockImplementationOnce((_cmd, cb) => cb(new Error()));
   	process.env.npm_config_prefix = undefined;
   	common.getInstallationPath(callback);
   	expect(callback).toHaveBeenCalledWith(new Error('Error finding binary installation directory'));
   	});

   Apparently the latest npm also adds the environment variable npm_config_local_prefix for the npm run ... command. This causes the test to fail.

   Therefore, not only the npm_config_prefix environment variable, but also the npm_config_local_prefix environment variable must be overwritten.

        it('should call callback with error if binaries path is not found', () => {
          childProcess.exec.mockImplementationOnce((_cmd, cb) => cb(new Error()));
   
          process.env.npm_config_prefix = undefined;
   +      process.env.npm_config_local_prefix = undefined;
   
          common.getInstallationPath(callback);
   
          expect(callback).toHaveBeenCalledWith(new Error('Error finding binary installation directory'));
        });

   Note
   However, a perfect way to avoid similar bugs would be to remove all environment variables with npm_ prefixes.

        beforeEach(() => {
          callback = jest.fn();
   
          _process = { ...global.process, env: { ...process.env } };
   +
   +      // Remove all environment variables prefixed with "npm_"
   +      process.env = Object.fromEntries(
   +        Object.entries(process.env)
   +          .filter(([key]) => !key.startsWith('npm_'))
   +      );
   +      process.env.npm_config_user_agent = 'npm/9.0.0';
        });

   or

        beforeEach(() => {
          callback = jest.fn();
   
          _process = { ...global.process, env: { ...process.env } };
   +
   +      // Remove all environment variables prefixed with "npm_"
   +      for (const key of Object.keys(process.env)) {
   +        if (key.startsWith('npm_')) {
   +          process.env[key] = undefined;
   +        }
   +      }
   +      process.env.npm_config_user_agent = 'npm/9.0.0';
        });

   I add the npm_config_user_agent environment variable here because without it, used-pm returns undefined. The code in src/common.js is broken when used-pm returns undefined. I believe this is a bug in go-npm, but there is no strong reason to fix this bug since the npm_config_user_agent environment variable is always present in situations where go-npm is run.

With these two changes, all tests should be successful. Sorry for not mentioning this in #9!

[mheob]

Perfectly explained. There is nothing for me to add. Thank you @sounisi5011

[andreynering]

Thank you @sounisi5011, that's really helpful! 🙏

I plan to work on it once I have some time available.

@sounisi5011 Let me know if you'd like to help maintain this package. I would be happy to give you write access to the repository.

[sounisi5011]

@andreynering Thank you!
My writing, including this reply, is machine translated by DeepL Pro (and proofread by my weak English skills).
If this is still okay with you, I would like to help with this package.

[andreynering]

The CI is fixed and all PRs merged 🎉

@sounisi5011 I just invited you to the repository 🚀 You can chat with me on Discord if you happen to need any help or want to discuss something. If you prefer email or just opening on issue here, it's fine as well.

I did some basic mainteinance and published a new release. One of the changes was a rename from master to main, so you'll have to manually rename it locally or just clone it again (which is easier).