Skip to content

Conversation

@fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Sep 25, 2025

before

 Test Files  208 passed | 17 skipped (225)
      Tests  1564 passed | 210 skipped (1777)
   Start at  23:27:10
   Duration  38.98s (transform 34.48s, setup 0ms, collect 86.81s, tests 356.47s, environment 18ms, prepare 673ms)

after

 Test Files  208 passed | 17 skipped (225)
      Tests  1564 passed | 210 skipped (1777)
   Start at  23:29:39
   Duration  28.56s (transform 28.88s, setup 0ms, collect 77.89s, tests 321.37s, environment 16ms, prepare 725ms)

Summary by CodeRabbit

  • Tests

    • Expanded syntax-error matching in file loader tests to accept an additional message, improving robustness across environments.
  • Chores

    • Updated linting tool to a newer minor version.
    • Adjusted workspace configuration to prefer rolldown-vite and exclude Vite-related packages from minimum release age checks.
    • Removed an unused dev dependency from the supertest package.

before

 Test Files  208 passed | 17 skipped (225)
      Tests  1564 passed | 210 skipped (1777)
   Start at  23:22:56
   Duration  29.70s (transform 31.81s, setup 0ms, collect 84.12s, tests 331.86s, environment 19ms, prepare 681ms)

after

 Test Files  208 passed | 17 skipped (225)
      Tests  1564 passed | 210 skipped (1777)
   Start at  23:24:18
   Duration  29.42s (transform 31.75s, setup 0ms, collect 81.94s, tests 329.88s, environment 18ms, prepare 674ms)
@fengmk2 fengmk2 requested a review from Copilot September 25, 2025 15:26
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Test assertion in file_loader.test.ts broadened to match an additional syntax error message. Removed a devDependency from packages/supertest/package.json. Updated workspace configuration: bumped oxlint, added vite and rolldown-vite to minimumReleaseAgeExclude, and added an overrides mapping vite to npm:rolldown-vite@^7.1.13.

Changes

Cohort / File(s) Summary
Tests
packages/core/test/loader/file_loader.test.ts
Broadened error-match pattern to include "A 'yield' expression is only allowed in a generator body" alongside existing patterns.
Package management
packages/supertest/package.json
Removed devDependency "@eggjs/bin": "workspace:*".
Workspace configuration
pnpm-workspace.yaml
Bumped oxlint ^1.17.0 → ^1.18.0; added vite and rolldown-vite to minimumReleaseAgeExclude; added overrides mapping vite → npm:rolldown-vite@^7.1.13.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paw—tests now read the skies,
Catching yields where syntax flies.
A dev dep hops out, light and neat,
Workspace trails swept, configs complete.
With oxlint fresh and vite in tow,
This bunny nods—onward we go! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly summarizes the primary change of configuring Vitest to use rolldown-vite for test tooling, matching the PR’s objectives and reflecting the main update without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch use-rolldow-vite-for-vitest

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @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 focuses on enhancing the project's testing infrastructure by integrating rolldown-vite to optimize vitest performance. It also includes routine dependency updates and minor adjustments to accommodate these changes, ensuring continued stability and efficiency.

Highlights

  • Vitest Integration with Rolldown-Vite: The project now utilizes rolldown-vite as an override for vite within the vitest testing framework, aiming to improve test performance. Initial benchmarks show a slight reduction in overall test duration.
  • Dependency Updates: The oxlint dependency has been upgraded from version 1.17.0 to 1.18.0, and detect-libc has been updated from 2.1.0 to 2.1.1. Additionally, lightningcss related entries have been updated to version 1.30.1.
  • Test Suite Adjustments: A test case in packages/core/test/loader/file_loader.test.ts has been updated to account for a new error message, 'A 'yield' expression is only allowed in a generator body', likely due to changes introduced by the rolldown-vite integration.
  • Development Dependency Cleanup: The @eggjs/bin development dependency has been removed from the packages/supertest/package.json file.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the testing configuration to use rolldown-vite instead of standard Vite for Vitest builds, achieving similar test performance with slightly improved metrics. The change introduces a build tool override to leverage rolldown's faster compilation.

Key changes:

  • Overrides Vite with rolldown-vite package for improved build performance
  • Updates oxlint version to latest patch release
  • Adjusts test error message patterns to accommodate rolldown's different syntax error reporting

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
pnpm-workspace.yaml Adds rolldown-vite override configuration and updates oxlint version
packages/supertest/package.json Removes unused @eggjs/bin development dependency
packages/core/test/loader/file_loader.test.ts Updates error message regex to handle rolldown's syntax error format
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request replaces vite with rolldown-vite for vitest across the workspace by using a pnpm override. The changes include updating a test to match new error messages and modifying dependency configurations. While the tests are passing, my main concern is the global nature of this override. It could affect other parts of the project that rely on vite, such as the documentation site build. I've added a comment to seek clarification on whether these areas have been tested.

@cloudflare-workers-and-pages
Copy link

Deploying egg with  Cloudflare Pages  Cloudflare Pages

Latest commit: 17aa703
Status:🚫  Build failed.

View logs

@cloudflare-workers-and-pages
Copy link

Deploying egg-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 17aa703
Status:🚫  Build failed.

View logs

@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.56%. Comparing base (fdbb014) to head (17aa703).
⚠️ Report is 1 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #5541      +/-   ##
==========================================
- Coverage   75.87%   75.56%   -0.31%     
==========================================
  Files         114      114              
  Lines        5770     3999    -1771     
  Branches     1151      586     -565     
==========================================
- Hits         4378     3022    -1356     
+ Misses       1212      908     -304     
+ Partials      180       69     -111     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fdbb014 and 17aa703.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • packages/core/test/loader/file_loader.test.ts (1 hunks)
  • packages/supertest/package.json (0 hunks)
  • pnpm-workspace.yaml (2 hunks)
💤 Files with no reviewable changes (1)
  • packages/supertest/package.json
🧰 Additional context used
📓 Path-based instructions (7)
pnpm-workspace.yaml

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Define dependency versions via the catalog section in pnpm-workspace.yaml and reference them with "catalog:"

Define and manage external dependency versions centrally in the catalog section of pnpm-workspace.yaml

Files:

  • pnpm-workspace.yaml
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/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/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/core/test/loader/file_loader.test.ts
{packages,plugins,tools,examples}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable TypeScript strict mode and follow TS best practices (explicit return types for public APIs, prefer interfaces over type aliases for object shapes, use readonly where appropriate, avoid any and use unknown when necessary)

Files:

  • packages/core/test/loader/file_loader.test.ts
{packages,plugins}/**/test/**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Vitest test files must follow the naming pattern test/**/*.test.ts and import test APIs from vitest with Node.js assert for assertions

Files:

  • packages/core/test/loader/file_loader.test.ts
{packages,plugins,tools}/**/test/**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

All test files should live under test/ and be suffixed with .test.ts

Files:

  • packages/core/test/loader/file_loader.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-25T15:21:16.381Z
Learning: Applies to {packages,plugins}/**/test/**/*.test.ts : Vitest test files must follow the naming pattern test/**/*.test.ts and import test APIs from vitest with Node.js assert for assertions
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-25T15:21:16.381Z
Learning: Applies to {packages,plugins}/**/vitest.config.ts : Each package (except egg-bin) must include a Vitest configuration file vitest.config.ts
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
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
⏰ 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). (8)
  • GitHub Check: Test packages (macos-latest, 22)
  • GitHub Check: Test egg-bin (ubuntu-latest, 22)
  • GitHub Check: Test egg-bin (ubuntu-latest, 24)
  • GitHub Check: Test packages (ubuntu-latest, 22)
  • GitHub Check: Test packages (windows-latest, 24)
  • GitHub Check: Test packages (windows-latest, 22)
  • GitHub Check: Test packages (macos-latest, 24)
  • GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (3)
pnpm-workspace.yaml (3)

130-130: LGTM: oxlint bump to ^1.18.0

No concerns from me.


174-176: LGTM: added vite and rolldown-vite to minimumReleaseAgeExclude

Appropriate for fast‑moving toolchain packages.


177-178: Workspace-wide vite→rolldown-vite override safe
No package.json or source imports reference Vite—override won’t affect non-test packages. Optionally add rolldown-vite@^7.1.13 to the catalog for version centralization.

@fengmk2 fengmk2 merged commit a065b88 into next Sep 25, 2025
21 of 25 checks passed
@fengmk2 fengmk2 deleted the use-rolldow-vite-for-vitest branch September 25, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants