Skip to content

Comments

chore: get tests running in CI#124

Open
dbjorge wants to merge 5 commits intoRSeidelsohn:masterfrom
dbjorge:reenable-tests
Open

chore: get tests running in CI#124
dbjorge wants to merge 5 commits intoRSeidelsohn:masterfrom
dbjorge:reenable-tests

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          

> license-checker-rseidelsohn@4.4.2 pretest
> npm run lint:fix


> license-checker-rseidelsohn@4.4.2 lint:fix
> npm run lint -- --fix


> license-checker-rseidelsohn@4.4.2 lint
> eslint --ext js . --fix


> license-checker-rseidelsohn@4.4.2 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