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

[WIP] feat(node): add child_process module #1097

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Conversation

sgammon
Copy link
Member

@sgammon sgammon commented Jul 20, 2024

Draft Powered by Pull Request Badge

Summary

Adds initial support for Node's child_process module. Relates to:

Task list

  • child_process.ChildProcess
    • It exists
    • (Implementation coming soon)
  • Synchronous Execution
    • execSync
    • execFileSync
    • spawnSync
  • Asynchronous Execution
    • exec
    • execFile
    • spawn
    • fork note: leaving for future pr
  • Tests for child_process
    • Basic object/method tests
    • Bad type tests
    • Native process kill on POSIX
    • Custom process signals on POSIX
    • Process kill tests (native)
    • Process kill tests (JVM)
    • Process interaction tests
    • Callbacks working + tested
    • Event flow for ChildProcess
      • exit event
      • close event
      • disconnect event
      • error event
      • message event
  • General functionality
    • Need a plan for fork
    • Need a plan for process channels

@sgammon sgammon added feature Large PRs or issues with full-blown features 🚧 WIP Works-in-progress. Blocks merge lang:javascript Issues relating to JavaScript api:node Node API and stdlib labels Jul 20, 2024
@sgammon sgammon added this to the Release R7: Beta 1 milestone Jul 20, 2024
@sgammon sgammon self-assigned this Jul 20, 2024
Copy link

codecov bot commented Jul 20, 2024

Codecov Report

Attention: Patch coverage is 64.98952% with 334 lines in your changes missing coverage. Please review.

Project coverage is 53.02%. Comparing base (652036f) to head (ecf6350).

Files with missing lines Patch % Lines
...vm/internals/node/childProcess/NodeChildProcess.kt 65.04% 118 Missing and 40 partials ⚠️
...ime/intrinsics/js/node/childProcess/ForkOptions.kt 0.00% 77 Missing ⚠️
...e/gvm/internals/node/childProcess/InterElideIPC.kt 0.00% 36 Missing ⚠️
...c/main/kotlin/elide/runtime/exec/GuestExecution.kt 0.00% 13 Missing ⚠️
...me/intrinsics/js/node/childProcess/SpawnOptions.kt 87.83% 1 Missing and 8 partials ⚠️
...intrinsics/js/node/childProcess/ExecSyncOptions.kt 87.69% 2 Missing and 6 partials ⚠️
...ntrinsics/js/node/childProcess/SpawnSyncOptions.kt 87.50% 2 Missing and 6 partials ⚠️
...ime/intrinsics/js/node/childProcess/ExecOptions.kt 87.71% 2 Missing and 5 partials ⚠️
...ackages/cli/src/main/kotlin/elide/tool/cli/main.kt 0.00% 6 Missing ⚠️
...ckages/cli/src/main/kotlin/elide/tool/cli/Elide.kt 0.00% 3 Missing ⚠️
... and 5 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1097      +/-   ##
==========================================
+ Coverage   52.56%   53.02%   +0.45%     
==========================================
  Files         413      412       -1     
  Lines       15055    15419     +364     
  Branches     2720     2923     +203     
==========================================
+ Hits         7914     8176     +262     
- Misses       6231     6257      +26     
- Partials      910      986      +76     
Flag Coverage Δ
jvm 53.02% <64.98%> (+0.45%) ⬆️
lib 53.02% <64.98%> (+0.45%) ⬆️
native ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...de/runtime/gvm/internals/node/events/NodeEvents.kt 47.85% <ø> (+0.77%) ⬆️
...elide/runtime/gvm/internals/node/path/NodePaths.kt 66.00% <100.00%> (+0.27%) ⬆️
...me/intrinsics/js/node/childProcess/ChildProcess.kt 100.00% <100.00%> (ø)
...nsics/js/node/childProcess/ChildProcessDefaults.kt 100.00% <100.00%> (ø)
...rc/main/kotlin/elide/runtime/exec/GuestExecutor.kt 0.00% <0.00%> (ø)
...de/runtime/gvm/internals/node/fs/NodeFilesystem.kt 65.42% <94.73%> (+2.18%) ⬆️
...tlin/elide/runtime/gvm/internals/ProcessManager.kt 61.53% <33.33%> (-8.47%) ⬇️
.../internals/node/childProcess/ChildProcessNative.kt 75.00% <75.00%> (ø)
...ckages/cli/src/main/kotlin/elide/tool/cli/Elide.kt 36.44% <0.00%> (-0.35%) ⬇️
...ime/intrinsics/js/node/childProcess/StdioConfig.kt 95.16% <95.16%> (ø)
... and 9 more

... and 12 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 652036f...ecf6350. Read the comment docs.

@sgammon sgammon force-pushed the feat/child-process branch 2 times, most recently from b2bfd39 to 6a4eaa3 Compare July 21, 2024 04:51
sgammon added a commit to elide-dev/docs that referenced this pull request Jul 21, 2024
- docs(js): add docs for `node:child_process`
- chore: various cleanups

Relates to elide-dev/elide#1097

Signed-off-by: Sam Gammon <[email protected]>
@sgammon sgammon force-pushed the feat/child-process branch 3 times, most recently from 57c9550 to 45a7609 Compare July 21, 2024 22:41
- feat: implement `child_process:execFile`
- test: add tests for `node:child_process`

Signed-off-by: Sam Gammon <[email protected]>
- feat: implement child process kill via native method
- test: improve testing of child process option types
- test: add tests for `posix` crate kill process

Signed-off-by: Sam Gammon <[email protected]>
sgammon and others added 22 commits July 25, 2024 18:15
Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
Copy link

sonarcloud bot commented Aug 18, 2024

@sgammon sgammon marked this pull request as ready for review September 2, 2024 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api:node Node API and stdlib feature Large PRs or issues with full-blown features lang:javascript Issues relating to JavaScript 🚧 WIP Works-in-progress. Blocks merge
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant