search

eclipse-langium / langium

Next-gen language engineering / DSL framework
https://langium.org/
MIT License
669 stars 62 forks source link

generator-langium: extended yeoman generator extension to offer parsing, linking, and validation test stubs (#1282) #1298

Closed sailingKieler closed 6 months ago

sailingKieler commented 7 months ago
dhuebner commented 7 months ago

@sailingKieler Running npm run test currently fails for me with:

 FAIL  packages/generator-langium/test/yeoman-generator.test.ts > Check yeoman generator works > 3 Should produce files for Vitest
Error: EACCES: permission denied, mkdir '/__yo-gen-testing'

Could it be that there are too many ..?

Dir name in my case is: __dirname: /workspace/langium/packages/generator-langium/ Test tries to write to: Generating into directory: /__yo-gen-testing

sailingKieler commented 7 months ago

Hi Dennis,

@sailingKieler Running npm run test currently fails for me with:

 FAIL  packages/generator-langium/test/yeoman-generator.test.ts > Check yeoman generator works > 3 Should produce files for Vitest
Error: EACCES: permission denied, mkdir '/__yo-gen-testing'

Could it be that there are too many ..?

Dir name in my case is: __dirname: /workspace/langium/packages/generator-langium/ Test tries to write to: Generating into directory: /__yo-gen-testing

The test is supposed to write __yo-gen-testing next to langium, i.e. to /workspace/__yo-gen-testing in order to not see the langium root node_modules. Did you test it on Gitpod?

dhuebner commented 7 months ago

@sailingKieler Yes, I tested it with GitPod. I think the tests should not create files outside the workspace in any (except temp) case. However it seems that there is an additional .. in '../../../../__yo-gen-testing' that can me omitted. Can you confirm?

sailingKieler commented 7 months ago

@dhuebner you're right, and I guess that's due to the introduction of https://github.com/eclipse-langium/langium/blob/02b3c92388bf6740cf1dc879f22c2b4ae5cd73f6/packages/generator-langium/test/yeoman-generator.test.ts#L13 I had to rebase on. I'll fix that! Thanks a lot for testing! 🙏

sailingKieler commented 7 months ago

@dhuebner feel free to give another try

sailingKieler commented 7 months ago

okay I dropped the options, and the Jest support, apparently I'm the only lonely wolf using it (in a commercial project).

Should it then be a question, at all? Maybe not.

kaisalmen commented 7 months ago

@spoenemann the second problem I will fix with a separate PR. The minor updates of monaco-vscode-api are pretty major and therefore this error surfaces here. There is https://github.com/CodinGame/monaco-vscode-api/issues/291

kaisalmen commented 7 months ago

⬆️ Fix is here: https://github.com/eclipse-langium/langium/pull/1317

@sailingKieler consider upgrading to vite v5 / vitest v1 before merging this PR.

sailingKieler commented 7 months ago

@sailingKieler consider upgrading to vite v5 / vitest v1 before merging this PR.

Wouldn't that be out of scope for this PR?

kaisalmen commented 7 months ago

Wouldn't that be out of scope for this PR?

Only for the generated output, not the whole project

sailingKieler commented 7 months ago

Only for the generated output, not the whole project

Of course 🤦🏽‍♂️ 😅

sailingKieler commented 7 months ago

consider upgrading to vite v5 / vitest v1 before merging this PR.

Tested upgrading to vitest ~1.0.0 but that requires Node.js v18, see https://github.com/vitest-dev/vitest/blob/66933c3a590fa5cff35e252a5f107b0f41d9248b/packages/vitest/package.json#L103-L105 and https://github.com/vitest-dev/vitest/blob/66933c3a590fa5cff35e252a5f107b0f41d9248b/packages/vitest/package.json#L111-L113

The latter causes a conflict during npm i:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR! 
npm ERR! While resolving: vitest@1.0.4
npm ERR! Found: @types/node@16.18.68
npm ERR! node_modules/@types/node
npm ERR!   dev @types/node@"~16.18.41" from the root project
npm ERR!   peerOptional @types/node@">= 14" from vite@4.4.12
npm ERR!   node_modules/vite
npm ERR!     dev vite@"~4.4.11" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peerOptional @types/node@"^18.0.0 || >=20.0.0" from vitest@1.0.4
npm ERR! node_modules/vitest
npm ERR!   dev vitest@"~1.0.0" from the root project
npm ERR! 
npm ERR! Conflicting peer dependency: @types/node@20.10.4
npm ERR! node_modules/@types/node
npm ERR!   peerOptional @types/node@"^18.0.0 || >=20.0.0" from vitest@1.0.4
npm ERR!   node_modules/vitest
npm ERR!     dev vitest@"~1.0.0" from the root project

Do you wanna switch to v18 as minimally required node.js version?

kaisalmen commented 7 months ago

Do you wanna switch to v18 as minimally required node.js version?

The question is why not. node 18 was the last LTS version. 20 is the current LTS version. 16 is outdated.

sailingKieler commented 7 months ago

Do you wanna switch to v18 as minimally required node.js version?

The question is why not. node 18 was the last LTS version. 20 is the current LTS version. 16 is outdated.

Filed dedicated PR https://github.com/eclipse-langium/langium/pull/1319

sailingKieler commented 6 months ago

@msujew @kaisalmen I upgraded the versions of node.js & vitest in the generated package.json, and fixed some further tiny bugs.

Can you guys have a final look?