egg-cluster icon indicating copy to clipboard operation
egg-cluster copied to clipboard

refactor: startMode

Open hyj1991 opened this issue 3 years ago • 4 comments

Checklist
  • [x] npm test passes
  • [ ] tests and/or benchmarks are included
  • [x] documentation is changed or added
  • [x] commit message follows commit guidelines
Affected core subsystem(s)

Deploy

Description of change

Reference: https://github.com/eggjs/egg/issues/4960

hyj1991 avatar Jun 06 '22 14:06 hyj1991

Codecov Report

Merging #100 (43cb9e8) into master (efa8441) will decrease coverage by 2.14%. The diff coverage is 92.41%.

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
- Coverage   98.96%   96.81%   -2.15%     
==========================================
  Files           8       13       +5     
  Lines         580      723     +143     
  Branches      105      112       +7     
==========================================
+ Hits          574      700     +126     
- Misses          6       23      +17     
Impacted Files Coverage Δ
lib/utils/options.js 97.50% <ø> (ø)
lib/utils/start_mode/worker_threads/agent.js 69.23% <69.23%> (ø)
lib/agent_worker.js 96.42% <85.71%> (-3.58%) :arrow_down:
lib/utils/start_mode/process/agent.js 93.22% <93.22%> (ø)
lib/app_worker.js 100.00% <100.00%> (ø)
lib/master.js 100.00% <100.00%> (+1.28%) :arrow_up:
lib/utils/manager.js 97.50% <100.00%> (+0.13%) :arrow_up:
lib/utils/messenger.js 100.00% <100.00%> (ø)
lib/utils/start_mode/process/app.js 100.00% <100.00%> (ø)
lib/utils/start_mode/worker_threads/app.js 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

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

codecov[bot] avatar Jun 07 '22 02:06 codecov[bot]

还有一些测试要补下

hyj1991 avatar Jun 10 '22 03:06 hyj1991

实际测试了下,child_process 中启动的 worker_threads 调用的其它文件指令会被 nyc 忽略:

  • https://github.com/eggjs/egg-cluster/pull/100/files#diff-70005bdc6996e6c69b89150d4829df5f33ecba8f279fc35c696d02cbcc6fd5adR15-R34

这里实际上已经被 lib/agent.jsworker_threads.test.js 的 case 中调用了,但是覆盖率不显示

hyj1991 avatar Jun 10 '22 12:06 hyj1991

麻烦问下这个pr现在可以合并了哇

oliverzy avatar Jul 30 '22 16:07 oliverzy

实际测试了下,child_process 中启动的 worker_threads 调用的其它文件指令会被 nyc 忽略:

  • https://github.com/eggjs/egg-cluster/pull/100/files#diff-70005bdc6996e6c69b89150d4829df5f33ecba8f279fc35c696d02cbcc6fd5adR15-R34

这里实际上已经被 lib/agent.jsworker_threads.test.js 的 case 中调用了,但是覆盖率不显示

换为 c8 试试,egg-bin 支持 c8 了

atian25 avatar Oct 29 '22 17:10 atian25

@hyj1991 下周再改改,我们发个 major。app worker 可以加一下线程模式,目前可以在 1 个 app worker 下使用,特别适合单测。

fengmk2 avatar Oct 29 '22 17:10 fengmk2

我来继续处理下这个 pr,如果不考虑多 worker 的形式,可以把另一半也补上

hyj1991 avatar Oct 30 '22 00:10 hyj1991

rebase 了下分支,有另一些改动有 conflict,会另开一个 PR 补上完整 worker(single) + agent 的实现

hyj1991 avatar Oct 30 '22 01:10 hyj1991