chore: move mock to plugins #5537
Conversation
WalkthroughDev tooling adds Vitest to cluster package devDependencies. Tests adjust imports to TypeScript sources, update mock paths to plugins/mock, and skip one cluster test. Documentation updates replace egg-mock with @eggjs/mock in both English and Chinese READMEs. No runtime code or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
c7a910d
|
| Status: | 🚫 Build failed. |
Deploying egg-v3 with
|
| Latest commit: |
c7a910d
|
| Status: | 🚫 Build failed. |
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring effort by relocating the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5537 +/- ##
==========================================
- Coverage 75.87% 75.84% -0.04%
==========================================
Files 114 114
Lines 5770 5770
Branches 1152 1149 -3
==========================================
- Hits 4378 4376 -2
- Misses 1212 1214 +2
Partials 180 180 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/mock/README.zh_CN.md (1)
13-15: Update lingeringegg-mockreferences
Replace all imports and mentions ofegg-mock(e.g.require('egg-mock/bootstrap'), docs links) with the new@eggjs/mockpackage across site/docs/tutorials, docs/core/unittest, quickstart guides, and create-egg templates.
🧹 Nitpick comments (7)
packages/utils/test/import.test.ts (2)
11-15: Prefer node:assert over Vitest expect in Vitest suites.
Repo convention uses built-in assert. Suggest replacing expect here.Apply this diff:
- it('should be true', () => { - expect(isESM).toBe(true); - }); + it('should be true', () => { + assert.equal(isESM, true); + });
186-193: Use node:assert for assertions (consistency).
Replace toBeDefined with assert.ok.- expect(obj.Agent).toBeDefined(); + assert.ok(obj.Agent);packages/cluster/test/app_worker.test.ts (1)
318-338: Make the skip actionable (reason/issue link) or guard with skipIf.
Add a short comment or use it.skipIf with a condition, mirroring the suite-level skip rationale.- it.skip('should refork when app_worker exit', async () => { + // TODO: flaky on CI (refork race). Re-enable after stabilization. Track: <issue-url> + it.skip('should refork when app_worker exit', async () => {packages/core/test/loader/file_loader.test.ts (1)
14-16: Test title no longer matches what’s exercised.
You’re passing a concrete src directory, not resolving via package.json exports. Either rename the test or switch to an exports-resolved path.Rename for clarity:
- it('should load files with package.json#exports', async () => { + it('should load files from plugin sources (mock middleware)', async () => {plugins/mock/README.md (3)
14-15: Update package name in description.
Use @eggjs/mock instead of egg-mock.-`egg-mock` inherits all APIs from [node_modules/mm](https://github.com/node-modules/mm), offering more flexibility. +`@eggjs/mock` inherits all APIs from [node_modules/mm](https://github.com/node-modules/mm), offering more flexibility.
422-422: Typo: “alse” → “also”.-You can alse mock agent.httpclient +You can also mock agent.httpclient
487-489: Doc consistency: replace egg-mock with @eggjs/mock.-And if you use mm.app to bootstrap app, you can manually call setGetAppCallback, -then egg-mock will inject ctx for each test case. +And if you use mm.app to bootstrap app, you can manually call setGetAppCallback, +then @eggjs/mock will inject ctx for each test case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
plugins/mock/test/__snapshots__/app_proxy.test.ts.snapis excluded by!**/*.snapplugins/mock/test/fixtures/yadan_app/node_modules/yadan/config/config.default.jsis excluded by!**/node_modules/**plugins/mock/test/fixtures/yadan_app/node_modules/yadan/index.jsis excluded by!**/node_modules/**plugins/mock/test/fixtures/yadan_app/node_modules/yadan/package.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (6)
packages/cluster/package.json(1 hunks)packages/cluster/test/app_worker.test.ts(1 hunks)packages/core/test/loader/file_loader.test.ts(1 hunks)packages/utils/test/import.test.ts(2 hunks)plugins/mock/README.md(1 hunks)plugins/mock/README.zh_CN.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/*/package.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use "workspace:*" for internal workspace dependencies in package.json
Files:
packages/cluster/package.json
{packages,plugins,tools}/**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins,tools}/**/package.json: Individual packages must reference external dependencies via "catalog:" from pnpm-workspace.yaml
Use "workspace:*" for internal monorepo dependencies between packages
Files:
packages/cluster/package.json
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Name test files as test/**/*.test.ts and run them with Vitest
Use import { describe, it } from 'vitest' in tests
Use Node.js built-in assert module for test assertions
Files:
packages/cluster/test/app_worker.test.tspackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
{packages,plugins}/**/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages,plugins}/**/test/**/*.test.ts: Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Use Node.js built-in assert for assertions in Vitest tests
Files:
packages/cluster/test/app_worker.test.tspackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
packages/cluster/test/app_worker.test.tspackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites
Files:
packages/cluster/test/app_worker.test.tspackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/vitest.config.ts : Each non-CLI package must include a vitest.config.ts for test configuration
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin (CLI tool) tests must use Mocha
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Use Node.js built-in assert for assertions in Vitest tests
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/vitest.config.ts : Each non-CLI package must include a vitest.config.ts for test configuration
Applied to files:
packages/cluster/package.jsonpackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Use Node.js built-in assert for assertions in Vitest tests
Applied to files:
packages/cluster/package.jsonpackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Vitest test files should follow the naming pattern test/**/*.test.ts and import from vitest
Applied to files:
packages/cluster/package.jsonpackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use import { describe, it } from 'vitest' in tests
Applied to files:
packages/cluster/package.jsonpackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Name test files as test/**/*.test.ts and run them with Vitest
Applied to files:
packages/cluster/package.jsonpackages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-15T01:56:16.297Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T01:56:16.297Z
Learning: Applies to tools/egg-bin/test/**/*.test.ts : egg-bin (CLI tool) tests must use Mocha
Applied to files:
plugins/mock/README.zh_CN.mdplugins/mock/README.md
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Place test suites following Vitest discovery: **/test/**/*.test.ts
Applied to files:
packages/utils/test/import.test.ts
📚 Learning: 2025-09-14T08:41:30.618Z
Learnt from: CR
PR: eggjs/egg#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-14T08:41:30.618Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in assert module for test assertions
Applied to files:
packages/utils/test/import.test.tspackages/core/test/loader/file_loader.test.ts
📚 Learning: 2025-09-18T07:56:17.417Z
Learnt from: CR
PR: eggjs/egg#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T07:56:17.417Z
Learning: Applies to **/test/**/*.test.ts : Mirror the repository test pattern when adding new suites
Applied to files:
packages/utils/test/import.test.ts
🧬 Code graph analysis (2)
packages/utils/test/import.test.ts (1)
packages/utils/src/import.ts (1)
importResolve(275-361)
packages/core/test/loader/file_loader.test.ts (1)
packages/core/test/helper.ts (1)
getFilepath(17-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Test egg-bin (ubuntu-latest, 24)
- GitHub Check: Test egg-bin (macos-latest, 22)
- GitHub Check: Test egg-bin (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (windows-latest, 24)
- GitHub Check: Test egg-bin (windows-latest, 22)
- GitHub Check: Test packages (ubuntu-latest, 22)
- GitHub Check: Test egg-bin (macos-latest, 24)
- GitHub Check: Test packages (windows-latest, 24)
- GitHub Check: Test packages (ubuntu-latest, 24)
- GitHub Check: Test packages (macos-latest, 24)
- GitHub Check: Test packages (macos-latest, 22)
- GitHub Check: Test packages (windows-latest, 22)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
plugins/mock/README.zh_CN.md (1)
19-19: LGTM: install command now points to @eggjs/mock.packages/utils/test/import.test.ts (2)
8-8: LGTM: helper import switched to .ts to match TS sources.
20-22: Confirm intent: resolving plugin source path under development.
Asserting resolve('plugins/mock/app') -> 'plugins/mock/src/app.ts' is correct for dev. Ensure this stays accurate if plugin build layout changes.packages/core/test/loader/file_loader.test.ts (1)
8-9: LGTM: tests now import TS sources and helpers.plugins/mock/README.md (1)
19-19: LGTM: install command updated to @eggjs/mock.packages/cluster/package.json (2)
74-76: LGTM: add vitest to devDependencies for this package.
Matches test script (“vitest run”) and test usage.
24-30: All non-CLI packages have vitest.config.ts
packages/cluster/vitest.config.ts exists, and no root packages under packages/ or plugins/ are missing a config (the only “missing” paths were test fixture apps, which don’t require their own vitest.config.ts).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily involves refactoring by moving the mock package into the plugins directory. The changes are mostly path updates across various files, which appear to be correct and consistent with the move. The dependencies have been updated to include vitest, and test imports now correctly reference TypeScript source files. Documentation has also been updated to reflect the new scoped package name @eggjs/mock.
I have one suggestion regarding a skipped test to ensure it's tracked for future resolution. Overall, this is a solid refactoring effort.
| }); | ||
|
|
||
| it('should refork when app_worker exit', async () => { | ||
| it.skip('should refork when app_worker exit', async () => { |
There was a problem hiding this comment.
While skipping a flaky test is a good temporary measure to stabilize the build, it's crucial to ensure it doesn't get forgotten. I've added a // TODO: comment to track the fix for this test. Please replace #XYZ with the actual issue number if one exists, or create one.
| it.skip('should refork when app_worker exit', async () => { | |
| // TODO: This test is flaky and has been temporarily skipped. See issue #XYZ for details. | |
| it.skip('should refork when app_worker exit', async () => { |
1 participant
Summary by CodeRabbit
Chores
Tests
Documentation