Skip to content

chore: get tests running in CI #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dbjorge
Copy link

@dbjorge dbjorge commented Oct 25, 2024

While I was looking at making a different contribution, I noticed that the repo's tests weren't in a running state. It looks like the tests were written prior to the project converting to "type": "module", but the tests were never updated to work with that. This PR:

  • Replaces jenkins-mocha and nyc with mocha and c8 to re-enable debugging and code coverage
  • Updates the tests to use ESM syntax instead of CJS syntax
  • Updates how packages-test.js was using child_process.spawn to fix how it was trying to read stdout
  • Updates test.ts cases that pin to the actual dependencies of the repo to account for the dependency updates
  • Adds a GitHub Actions configuration to run lint + format + test checks on PRs

@Standard8
Copy link

@dbjorge I'm not the maintainer, but I was looking at this patch, and when I run npm ci locally and then npm test, I get the following output:

% npm test          

> [email protected] pretest
> npm run lint:fix


> [email protected] lint:fix
> npm run lint -- --fix


> [email protected] lint
> eslint --ext js . --fix


> [email protected] test
> mocha ./tests/*.js

✔ clarifications (0.279458ms)

 Exception during run: SyntaxError[ @/Users/mark/dev/license-checker-rseidelsohn/tests/test.js ]: Unexpected identifier 'assert'
    at compileSourceTextModule (node:internal/modules/esm/utils:338:16)
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:106:18)
    at #translate (node:internal/modules/esm/loader:470:12)
    at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:517:27)
    at async ModuleJob._link (node:internal/modules/esm/module_job:115:19)

Any ideas? Could it be to do with a newer node version?

@dbjorge
Copy link
Author

dbjorge commented Jan 10, 2025

Could it be to do with a newer node version?

Maybe; I can't say without knowing what version you're trying to run against. The version I tested against is Node 18 (because it is the earliest version this library's package.json engines field indicates support for) - if you're trying to test against an older version, you should try using that instead.

@dbjorge
Copy link
Author

dbjorge commented Jan 10, 2025

@RSeidelsohn , do you anticipate being able to review this? If not I'll go ahead and close it to clean up my list of outstanding work.

@RSeidelsohn
Copy link
Owner

Hello @dbjorge ,

thanks a lot for your valuable contribution! Please give me more time to tackle this. I came back to actually maintaining the code base after months of inactivity and am currently in the middle of a bigger refactoring. After that, I'll look into your PR and will do my best checking it and resolving probably merge conflicts. Will send a ping when done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants