test(bundler): add tegg-app fixture with HTTPController + service#5887
test(bundler): add tegg-app fixture with HTTPController + service#5887killagu wants to merge 1 commit intosplit/15-bundler-no-fs-scan-testfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new test fixture, tegg-app, to the egg-bundler tool to facilitate integration testing for applications using the Tegg framework. The changes include adding necessary dependencies to package.json, setting up the fixture's configuration, and implementing a sample module with a controller and service. Review feedback suggests improving TypeScript type safety in the new controller by using definite assignment assertions for injected properties and marking query parameters as optional to handle missing values correctly.
| @Inject() | ||
| fooService: FooService; |
There was a problem hiding this comment.
In TypeScript, when using @Inject() without a constructor, the property should be marked with the definite assignment assertion operator (!) to avoid "Property has no initializer" errors when strictPropertyInitialization is enabled in the compiler options.
| @Inject() | |
| fooService: FooService; | |
| @Inject() | |
| fooService!: FooService; |
| method: HTTPMethodEnum.GET, | ||
| path: '/hello', | ||
| }) | ||
| async hello(@HTTPQuery() name: string): Promise<{ message: string }> { |
There was a problem hiding this comment.
The @httpquery() parameter name can be undefined if the query parameter is missing from the request. It should be marked as optional (name?: string) to be type-safe and avoid potential compilation errors in strict mode.
| async hello(@HTTPQuery() name: string): Promise<{ message: string }> { | |
| async hello(@HTTPQuery() name?: string): Promise<{ message: string }> { |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new tegg-app test fixture and a small integration test to pin its on-disk module/controller/service layout, enabling future bundler integration coverage of the tegg plugin path.
Changes:
- Add
test/fixtures/apps/tegg-app/with Egg + tegg plugin config, module metadata, and a controller/service pair. - Add an integration test that asserts the fixture’s expected file structure exists.
- Add tegg-related workspace devDependencies to
tools/egg-bundlerto support fixture usage in tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/egg-bundler/test/integration.test.ts | Adds a structure test to assert the tegg fixture’s expected files exist. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/tsconfig.json | Introduces a tsconfig for the new tegg-app fixture extending repo defaults. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/package.json | Adds a fixture package manifest with tegg + egg dependencies. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/modules/foo/package.json | Declares an Egg module package for the foo module. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/modules/foo/FooService.ts | Adds a @SingletonProto tegg service for the fixture. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/modules/foo/FooController.ts | Adds a @HTTPController with GET /tegg/hello for the fixture. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/config/plugin.ts | Enables tegg plugins (tegg-config, tegg-plugin, controller-plugin) for the fixture app. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/config/module.json | Registers the foo module for tegg. |
| tools/egg-bundler/test/fixtures/apps/tegg-app/config/config.default.ts | Adds minimal Egg config for the fixture (keys, CSRF disabled). |
| tools/egg-bundler/package.json | Adds tegg-related packages to devDependencies for tests/tooling. |
| }) | ||
| export class FooController { | ||
| @Inject() | ||
| fooService: FooService; |
| method: HTTPMethodEnum.GET, | ||
| path: '/hello', | ||
| }) | ||
| async hello(@HTTPQuery() name: string): Promise<{ message: string }> { |
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "extends": "../../../../../../tsconfig.json" | |||
Adds
test/fixtures/apps/tegg-app/with@SingletonProtoservice +@HTTPControllerexposingGET /tegg/hello. Exercises the tegg plugin path in integration tests alongside minimal-app/empty-app.Part of #5863 split. Tracking: #5871.
🤖 Generated with Claude Code