Skip to content

Start Moving Tests to JS - #381

Draft
lishaduck wants to merge 4 commits into
jfmengels:mainfrom
lishaduck:big-refactor-pt-4
Draft

Start Moving Tests to JS#381
lishaduck wants to merge 4 commits into
jfmengels:mainfrom
lishaduck:big-refactor-pt-4

Conversation

@lishaduck

@lishaduck lishaduck commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

Blocked on refactoring cwd (#382)

Depends on #377.

Comment thread bin/elm-review

// TODO(@lishaduck): With ESM, this should use TLA.
require('../lib/main')();
require('../lib/main').main();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just thinking, we should ensure this doesn't break elmls.
As far as I can tell, it's just shelling out, correct?
https://github.com/elm-tooling/elm-language-server/blob/d0c9186ec7561abdf7d970be51767304b91d7f5b/src/common/providers/diagnostics/elmReviewDiagnostics.ts#L145-L150

@lishaduck
lishaduck force-pushed the big-refactor-pt-4 branch 5 times, most recently from 7ac5b1d to ed634cc Compare September 28, 2025 02:45
@lishaduck

Copy link
Copy Markdown
Contributor Author

#151 is really confusing me... node-exit shouldn't be needed since node v0.12, which has been EOL for forever. I assume that means that there are other bugs in process.exit() on Windows, but I'm kinda lost.
Currently refactoring away process.exit() into exceptions, we'll see how well that works.

@lishaduck lishaduck mentioned this pull request Oct 7, 2025
6 tasks
It works sometimes, and sometimes the `processExitSpy` gets bypassed at random.
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.

1 participant