Skip to content
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

Add testing setup #5

Merged
merged 5 commits into from
Feb 22, 2024
Merged

Add testing setup #5

merged 5 commits into from
Feb 22, 2024

Conversation

kevinramharak
Copy link
Contributor

Fixes #4

This PR implements:

  • using vitest as a test runner with its browser mode
  • a check on GameConfig.banner !== false before logging the phavuer banner to the console
  • some basic tests for the Game component

Notes:
The addition of:

  "resolutions": {
    "string-width": "^4.2.2"
  }

is needed because of isaacs/jackspeak#5.
Long story, but it comes down to require/esm being a huge mess when coupled with npm dependency hell.

@kevinramharak kevinramharak mentioned this pull request Feb 20, 2024
@laineus
Copy link
Owner

laineus commented Feb 21, 2024

@kevinramharak Thank you!
I haven't been able to confirm the details yet, but it seems that the following error occurs when I run yarn run test.

$ yarn run test
yarn run v1.22.10
$ vitest

 DEV  v1.3.1 /home/laineus/works/phavuer-kevin
      Browser runner started at http://localhost:5173/


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: require() of ES Module /home/laineus/works/phavuer-kevin/node_modules/wrap-ansi/index.js from /home/laineus/works/phavuer-kevin/node_modules/cliui/build/index.cjs not supported.
Instead change the require of index.js in /home/laineus/works/phavuer-kevin/node_modules/cliui/build/index.cjs to a dynamic import() which is available in all CommonJS modules.
 ❯ Object.<anonymous> node_modules/cliui/build/index.cjs:293:14

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'ERR_REQUIRE_ESM' }



error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@kevinramharak
Copy link
Contributor Author

@laineus I'll take another look, I noticed weird behaviour based on whether node_modules was already present or not. There are 3 packages that have this ESM/CJS mess. I think all of them are a dependency of cliui so maybe I can pin that to a working version.

@kevinramharak
Copy link
Contributor Author

@laineus This is caused by a resolution bug in yarn v1. The package jackspeak updated cliui to ^8.0.1 here which causes this mess, although its really a yarn v1 problem that is fixed in version 2.

I pinned jackspeak to 2.1.1 as suggested here, that seems to work, you might have to remove node_modules once because it seems yarn is stubborn on keeping the old broken packages even though the package.json and yarn.lock list different versions.

@laineus
Copy link
Owner

laineus commented Feb 22, 2024

@kevinramharak It's working correctly now!
Since there are no issues, I'll merge it.

Thank you for the great work!

@laineus laineus merged commit aa48de5 into laineus:master Feb 22, 2024
@kevinramharak kevinramharak deleted the feature/add-tests-vitest branch February 22, 2024 14:08
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.

Add testing setup
2 participants