Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/egg/src/lib/loader/AppWorkerLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export class AppWorkerLoader extends EggApplicationLoader {
// app
await this.loadController();
// app
await this.loadRouter(); // Depend on controllers
if (!this.options.metadataOnly) {
await this.loadRouter(); // Depend on controllers
}
}
}
38 changes: 38 additions & 0 deletions packages/egg/test/fixtures/apps/metadata-only-app/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
module.exports = class MetadataOnlyBoot {
constructor(app) {
this.app = app;
app.bootLog = [];
}

configWillLoad() {
this.app.bootLog.push('configWillLoad');
}

configDidLoad() {
this.app.bootLog.push('configDidLoad');
}

async didLoad() {
this.app.bootLog.push('didLoad');
}

async willReady() {
this.app.bootLog.push('willReady');
}

async didReady() {
this.app.bootLog.push('didReady');
}

async serverDidReady() {
this.app.bootLog.push('serverDidReady');
}

async beforeClose() {
this.app.bootLog.push('beforeClose');
}

async loadMetadata() {
this.app.bootLog.push('loadMetadata');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = async function () {
this.body = 'hello';
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = (app) => {
app.get('/', app.controller.home);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "metadata-only-app"
}
116 changes: 54 additions & 62 deletions packages/egg/test/start.test.ts
Original file line number Diff line number Diff line change
@@ -1,66 +1,58 @@
import { describe } from 'vitest';
import { strict as assert } from 'node:assert';

// import utils from '../utils';
// import assert from 'assert';
// import path from 'path';
import { describe, it, afterAll, beforeAll } from 'vitest';

// let app;
import { singleProcessApp, type SingleModeApplication } from './utils.ts';

describe.skip('test/lib/start.test.js', () => {
// afterEach(() => app.close());
// describe('start', () => {
// it('should dump config and plugins', async () => {
// app = await utils.singleProcessApp('apps/demo');
// const baseDir = utils.getFilepath('apps/demo');
// let json = require(path.join(baseDir, 'run/agent_config.json'));
// assert(/\d+\.\d+\.\d+/.test(json.plugins.onerror.version));
// assert(json.config.name === 'demo');
// assert(json.config.tips === 'hello egg');
// json = require(path.join(baseDir, 'run/application_config.json'));
// checkApp(json);
// const dumpped = app.dumpConfigToObject();
// checkApp(dumpped.config);
// function checkApp(json) {
// assert(/\d+\.\d+\.\d+/.test(json.plugins.onerror.version));
// assert(json.config.name === 'demo');
// // should dump dynamic config
// assert(json.config.tips === 'hello egg started');
// }
// });
// it('should request work', async () => {
// app = await utils.singleProcessApp('apps/demo');
// await app.httpRequest().get('/protocol')
// .expect(200)
// .expect('http');
// await app.httpRequest().get('/class-controller')
// .expect(200)
// .expect('this is bar!');
// });
// it('should env work', async () => {
// app = await utils.singleProcessApp('apps/demo', { env: 'prod' });
// assert(app.config.env === 'prod');
// });
// });
// describe('custom framework work', () => {
// it('should work with options.framework', async () => {
// app = await utils.singleProcessApp('apps/demo', { framework: path.join(__dirname, '../fixtures/custom-egg') });
// assert(app.customEgg);
// await app.httpRequest().get('/protocol')
// .expect(200)
// .expect('http');
// await app.httpRequest().get('/class-controller')
// .expect(200)
// .expect('this is bar!');
// });
// it('should work with package.egg.framework', async () => {
// app = await utils.singleProcessApp('apps/custom-framework-demo');
// assert(app.customEgg);
// await app.httpRequest().get('/protocol')
// .expect(200)
// .expect('http');
// await app.httpRequest().get('/class-controller')
// .expect(200)
// .expect('this is bar!');
// });
// });
describe('test/start.test.ts', () => {
describe('metadataOnly mode', () => {
let app: SingleModeApplication;

beforeAll(async () => {
app = await singleProcessApp('apps/metadata-only-app', { metadataOnly: true });
});

afterAll(async () => {
await app.close();
});
Comment on lines +15 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard teardown against setup failure in afterAll.

If beforeAll fails, app can be unset and app.close() throws, which can mask the original failure (Line 16 and Line 40).

🛠️ Suggested fix
-    let app: SingleModeApplication;
+    let app: SingleModeApplication | undefined;
@@
     afterAll(async () => {
-      await app.close();
+      if (app) await app.close();
     });
@@
-    let app: SingleModeApplication;
+    let app: SingleModeApplication | undefined;
@@
     afterAll(async () => {
-      await app.close();
+      if (app) await app.close();
     });

Also applies to: 39-41

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/egg/test/start.test.ts` around lines 15 - 17, The teardown in the
afterAll blocks calls await app.close() unguarded and will throw if beforeAll
failed to set app; update both afterAll handlers to check that the test App
instance exists before calling close (e.g., if (app) await app.close() or use
optional chaining) so teardown is skipped when setup failed; locate the afterAll
closures in this test file (referencing the app variable used in
beforeAll/afterAll) and add the existence guard in both places.


it('should only call loadMetadata, not normal lifecycle hooks', () => {
assert.deepStrictEqual(app.bootLog, ['loadMetadata']);
});

it('should skip loadRouter — no routes registered', () => {
assert.strictEqual(app.router.stack.length, 0);
});

it('should not create agent', () => {
assert.strictEqual(app.agent, undefined);
});
Comment on lines +27 to +29
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

In this test app is typed as SingleModeApplication (from test/utils.ts), where agent is declared as a required property. But here metadataOnly mode explicitly expects app.agent to be undefined, so the type annotation is misleading and reduces type-safety for future edits. Consider using a local type with agent?: for metadataOnly tests, or update the shared SingleModeApplication typing to reflect the metadataOnly case (e.g. conditional/union type or separate SingleModeApplicationMetadataOnly).

Copilot uses AI. Check for mistakes.
});

describe('normal mode (baseline)', () => {
let app: SingleModeApplication;

beforeAll(async () => {
app = await singleProcessApp('apps/metadata-only-app');
});

afterAll(async () => {
await app.close();
});

it('should call normal lifecycle hooks, not loadMetadata', () => {
assert(app.bootLog.includes('configDidLoad'));
assert(app.bootLog.includes('didLoad'));
assert(app.bootLog.includes('willReady'));
assert(!app.bootLog.includes('loadMetadata'));
});

it('should register routes', () => {
assert(app.router.stack.length > 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

你好,这里的断言可以更精确。我们明确知道 metadata-only-app 这个测试应用只注册了一个路由,所以可以直接判断 app.router.stack.length 是否等于 1,而不是 > 0。这样可以让测试用例更加健壮。

Suggested change
assert(app.router.stack.length > 0);
assert.strictEqual(app.router.stack.length, 1);

});

it('should create agent', () => {
assert(app.agent);
});
});
});
Loading