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

fix: app should wait for agent ready on parallel mode #144

Merged
merged 9 commits into from
Jan 11, 2023

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Jan 10, 2023

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

@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Base: 82.57% // Head: 82.11% // Decreases project coverage by -0.45% ⚠️

Coverage data is based on head (7eb3d42) compared to base (880c282).
Patch coverage: 56.09% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #144      +/-   ##
==========================================
- Coverage   82.57%   82.11%   -0.46%     
==========================================
  Files          25       25              
  Lines        2341     2371      +30     
  Branches      380      381       +1     
==========================================
+ Hits         1933     1947      +14     
- Misses        408      424      +16     
Impacted Files Coverage Δ
lib/parallel/app.js 28.34% <20.00%> (-0.69%) ⬇️
bootstrap.js 65.90% <28.57%> (-15.91%) ⬇️
lib/agent.js 69.23% <70.00%> (-3.00%) ⬇️
register.js 88.88% <88.88%> (-0.31%) ⬇️
lib/app.js 96.91% <100.00%> (ø)
lib/parallel/agent.js 27.48% <100.00%> (ø)
lib/parallel/util.js 38.46% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

bootstrap.js Outdated
if (process.env.ENABLE_MOCHA_PARALLEL && process.env.AUTO_AGENT) {
// setup agent first
agent.setupAgent();
Copy link
Member Author

Choose a reason for hiding this comment

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

得手动 setup

Copy link
Member Author

Choose a reason for hiding this comment

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

这样也不对,按道理应该先执行 mochaGlobalSetup 才对的

beforeInit: async _app => {
const agent = await setupAgent();
_app.options.clusterPort = agent.options.clusterPort;
debug('mockParallelApp beforeInit get clusterPort: %s', _app.options.clusterPort);
Copy link
Member Author

Choose a reason for hiding this comment

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

image

终于成功了,mocha global hook 有时序问题

@fengmk2 fengmk2 changed the title fix: debug beta fix: app should wait for agent ready on parallel mode Jan 10, 2023
@fengmk2 fengmk2 marked this pull request as ready for review January 10, 2023 18:18
@fengmk2 fengmk2 merged commit 205e836 into master Jan 11, 2023
@fengmk2 fengmk2 deleted the fix-app-CLUSTER_PORT-missing branch January 11, 2023 01:19
atian25 pushed a commit that referenced this pull request Jan 11, 2023
[skip ci]

## [5.8.3](v5.8.2...v5.8.3) (2023-01-11)

### Bug Fixes

* app should wait for agent ready on parallel mode ([#144](#144)) ([205e836](205e836))
@github-actions
Copy link

🎉 This PR is included in version 5.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

fengmk2 added a commit to eggjs/egg-bin that referenced this pull request Jan 11, 2023
@fengmk2
Copy link
Member Author

fengmk2 commented Jan 11, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant