fix: auto set mocha @eggjs/mock/register on application project type #281
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces changes to the Egg.js testing framework, enhancing the test command functionality and adding support for ECMAScript Module (ESM) environments. Key updates include modifications to the Changes
Sequence DiagramsequenceDiagram
participant TestCommand as Test Command
participant EggUtils as @eggjs/utils
participant MockRegister as @eggjs/mock/register
participant TestRunner as Mocha Test Runner
TestCommand->>EggUtils: Detect Application Type
EggUtils-->>TestCommand: Return Application Type
alt Is Application Type
TestCommand->>MockRegister: Attempt to Register
MockRegister-->>TestCommand: Registration Result
end
TestCommand->>TestRunner: Configure and Run Tests
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@eggjs/[email protected] |
commit: |
![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
🧹 Nitpick comments (2)
src/commands/test.ts (1)
118-130: Consider providing a gentle warning for missing@eggjs/mock
This logic properly handles the scenario where@eggjs/mock/registermay not be installed. However, ignoring the exception silently might confuse end users expecting mock features to be automatically available. Consider logging a user-friendly info or warning when an installation is missing.test/fixtures/test-demo-app-esm/test/a.test.js (1)
1-10: Clear and concise test coverage
This test effectively verifies the main route’s output. Consider adding negative or edge-case tests for broader coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/fixtures/test-demo-app-esm/node_modules/eggis excluded by!**/node_modules/**
📒 Files selected for processing (10)
.gitignore(1 hunks)package.json(3 hunks)src/commands/test.ts(2 hunks)test/commands/test.test.ts(1 hunks)test/fixtures/test-demo-app-esm/app/router.js(1 hunks)test/fixtures/test-demo-app-esm/config/config.default.js(1 hunks)test/fixtures/test-demo-app-esm/package.json(1 hunks)test/fixtures/test-demo-app-esm/test/a.test.js(1 hunks)test/fixtures/test-demo-app/app/router.js(0 hunks)test/fixtures/test-demo-app/test/a.test.js(0 hunks)
💤 Files with no reviewable changes (2)
- test/fixtures/test-demo-app/app/router.js
- test/fixtures/test-demo-app/test/a.test.js
✅ Files skipped from review due to trivial changes (2)
- test/fixtures/test-demo-app-esm/config/config.default.js
- test/fixtures/test-demo-app-esm/package.json
🔇 Additional comments (8)
src/commands/test.ts (1)
7-7: Neat import for advanced Egg detection
Bringing in importResolve, detectType, and EggType from @eggjs/utils is a clean approach to manage application type detection and module resolution.
test/fixtures/test-demo-app-esm/app/router.js (1)
1-8: Simple and effective route definition
The implementation is straightforward. Setting this.body in an async function is consistent with Koa-like contexts. Looks good!
test/commands/test.test.ts (2)
23-32: Solid test for automatic mock registration in CommonJS
The test properly verifies that @eggjs/mock/register is auto-required when running in CommonJS mode and checks for expected output. Great job!
34-43: Robust test for ESM auto-require flow
Similar to the CommonJS test, this ensures the ESM flow automatically loads @eggjs/mock/register. Perfect addition for coverage.
.gitignore (1)
21-21: Allowing ESM fixture dependencies
Excluding node_modules everywhere except for test-demo-app-esm is a practical choice for maintaining a self-contained test fixture.
package.json (3)
21-21: LGTM: Utils dependency upgrade supports new functionality.
The upgrade of @eggjs/utils to ^4.2.0 aligns with the new functionality that uses detectType and EggType imports.
Line range hint 40-44: Good: Optional peer dependency configuration.
Making @eggjs/mock an optional peer dependency is a good practice as it allows flexibility in test configurations while still maintaining compatibility when the package is needed.
38-38: Verify the stability of @eggjs/mock v6.
The change from beta to v6 for @eggjs/mock is a significant upgrade. Since this package is critical for the auto-registration feature, let's verify its stability.
Also applies to: 47-47
✅ Verification successful
@eggjs/mock v6 appears stable and actively maintained
The package shows positive stability indicators:
- Latest release (v6.0.2) was published recently on December 29, 2024
- The release contains a minor bug fix for bootstrap app global setting
- Only one open issue exists, related to TypeScript type definitions
- The package has reached a stable v6 version number, moving past beta
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the stability and compatibility of @eggjs/mock v6
# Check the package's publish date and release notes
gh api graphql -f query='
{
repository(owner: "eggjs", name: "mock") {
releases(first: 1, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
tagName
publishedAt
description
}
}
}
}'
# Check for any recent issues or bug reports
gh api graphql -f query='
{
repository(owner: "eggjs", name: "mock") {
issues(first: 5, states: OPEN, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
title
createdAt
labels(first: 5) {
nodes {
name
}
}
}
}
}
}'
Length of output: 1239
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #281 +/- ##
==========================================
- Coverage 95.80% 95.44% -0.37%
==========================================
Files 21 21
Lines 1050 1054 +4
Branches 174 176 +2
==========================================
Hits 1006 1006
- Misses 44 48 +4 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [7.0.1](v7.0.0...v7.0.1) (2024-12-29) ### Bug Fixes * auto set mocha @eggjs/mock/register on application project type ([#281](#281)) ([929c0f8](929c0f8))
Summary by CodeRabbit
New Features
@eggjs/mock/registerin test environments.Bug Fixes
Documentation
Chores
.gitignoreto allow tracking specificnode_modulesdirectory.package.jsonfor the demo app.