[bugfix] Executing pnpm test on workspace folder with a space on the name doesn't work ENOENT: no such file or directory

[alexrecuenco]

Changes

• My workspace folder name External Projecs has a space. Executing pnpm test it executes promisifySpawn with shell:true. However, any arguments of child_process.spawn is not escaped when shell===true.
• I believe that shell: true might be unnecessary. If it isn't, another solution would have to be to do manual escaping of each command argument...

packages/vscode
astro-vscode:test: > node scripts/build-grammar.mjs
astro-vscode:test: 
astro-vscode:test: [09:54 AM] ✔ updated /Workspace/External Projects/language-tools/packages/vscode/syntaxes/astro.tmLanguage.json
astro-vscode:test: node:fs:581
astro-vscode:test:   return binding.open(
astro-vscode:test:                  ^
astro-vscode:test: 
astro-vscode:test: Error: ENOENT: no such file or directory, open '/Workspace/External'

Testing

If the tests don't run successfully on the test matrix on all systems, this is clearly wrong.

Docs

bug fix only on a test script

[changeset-bot]

⚠️ No Changeset found

Latest commit: 722b939

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Princesseuh]

shell: true is necessary because of nodejs/node#52554, will run tests to show the failure on Windows without it

[alexrecuenco]

I see. I imagined it had to be something to do with windows, but I would have never guessed that issue! In that case, I'll do the space quoting when I have some time.

Should I keep shell:true if the platform is not windows? It might be unnecessary.

https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows

[Princesseuh]

That seems fine to me!

[alexrecuenco]

Hi @Princesseuh , I haven't had much time. I would need some help to test this, since I don't have a windows machine. I think what I did is sane? at the very least better (assuming I followed the instructions correctly).

If it is too complex, we could just disable the escape in non-windows and leave that as a TODO for when someone with a windows machine encounters that issue and has a bit of time to fix it

This is unrelated, I think? but when I run pnpm test all the Content Intellisense - tests fail, they all receive a []. Is there something I should be doing on my setup to get those tests to work?

[Princesseuh]

Yeah, there's a failure on main on a content intellisense test, haven't had the time to investigate yet, but it's definitely unrelated.

[Princesseuh]

Looks great, thank you! Apologies for the delay.