fix: fix mocha failed title for inject_ctx (#162)

this.fail is only safe in runTests and should not be called in runSuite.
eggjs · Jun 21, 2023 · f6f59ac · f6f59ac
1 parent 2459c0d
commit f6f59ac
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 16 deletions.
diff --git a/lib/inject_context.js b/lib/inject_context.js
@@ -13,7 +13,7 @@ function injectContext(mocha) {
     return;
   }
 
-  const Runner = mocha.Runner;
+  const { Runner } = mocha;
   const runSuite = Runner.prototype.runSuite;
   const runTests = Runner.prototype.runTests;
 
@@ -29,21 +29,22 @@ function injectContext(mocha) {
   Runner.prototype.runSuite = async function(suite, fn) {
     debug('run suite: %s', suite.title);
     let app;
+    const self = this;
     try {
       app = await appHandler.getApp(suite);
       debug('get app: %s', !!app);
-    } catch (err) {
-      err.message = `[egg-mock/runSuite] get app for ${getTestTitle(suite)}: ${err.message}`;
-      this.fail(suite, err);
-      return fn(suite);
+      await app.ready();
+    } catch {
+      // 可能 app.ready 时报错，不使用失败的 app
+      app = null;
     }
-    const self = this;
     if (!app) {
+      // app 不存在，直接跳过，在 beforeEach 的 hook 中会报错
+      // 确保不打乱 mocha 的顺序，防止 mocha 内部状态错误
       return runSuite.call(self, suite, fn);
     }
     let errSuite;
     try {
-      await app.ready();
       suite.ctx[MOCHA_SUITE_APP] = app;
       const mockContextFun = app.mockModuleContextScope || app.mockContextScope;
       await mockContextFun.call(app, async function() {
@@ -55,8 +56,14 @@ function injectContext(mocha) {
         });
       });
     } catch (err) {
-      err.message = `[egg-mock/runSuite] inject context for ${getTestTitle(suite)}: ${err.message}`;
-      self.fail(suite, err);
+      // mockContext 失败后动态注册一个 beforeAll hook
+      // 快速失败，直接阻塞后续用例
+      suite.beforeAll('egg-mock-mock-ctx-failed', async () => {
+        throw err;
+      });
+      return runSuite.call(self, suite, aErrSuite => {
+        return fn(aErrSuite);
+      });
     }
     return fn(errSuite);
   };
@@ -94,7 +101,6 @@ function injectContext(mocha) {
         assert(app, `not found app for test ${getTestTitle(suite, test)}`);
         await app.ready();
       } catch (err) {
-        err.message = `[egg-mock/runTests] get app for ${getTestTitle(suite, test)}: ${err.message}`;
         self.fail(test, err);
         return next(i + 1);
       }
@@ -109,14 +115,12 @@ function injectContext(mocha) {
           });
         });
       } catch (err) {
-        err.message = `[egg-mock/runTests] create context for ${getTestTitle(suite)} error: ${err.message}`;
         self.fail(test, err);
         return next(i + 1);
       }
       return next(i + 1);
     }
     next(0).catch(err => {
-      err.message = `[egg-mock/runTests] unknown error ${getTestTitle(suite)} error: ${err.message}`;
       self.fail(suite, err);
       done(suite);
     });

diff --git a/test/fixtures/app-ready-failed/app.js b/test/fixtures/app-ready-failed/app.js
@@ -0,0 +1,6 @@
+module.exports = class AppHook {
+
+  async didLoad() {
+    throw new Error('mock app ready failed');
+  }
+};
diff --git a/test/fixtures/app-ready-failed/config/config.default.js b/test/fixtures/app-ready-failed/config/config.default.js
@@ -0,0 +1,5 @@
+'use strict';
+
+module.exports = {
+  keys: '123',
+};
diff --git a/test/fixtures/app-ready-failed/package.json b/test/fixtures/app-ready-failed/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "app"
+}
diff --git a/test/fixtures/app-ready-failed/test/index.test.js b/test/fixtures/app-ready-failed/test/index.test.js
@@ -0,0 +1,8 @@
+const assert = require('assert');
+const { app } = require('../../../../bootstrap');
+describe('test for app ready failed', () => {
+  it('should not print', () => {
+    // ...
+    assert(app);
+  });
+});
diff --git a/test/fixtures/create-context-failed/test/index.test.js b/test/fixtures/create-context-failed/test/index.test.js
@@ -8,6 +8,12 @@ setGetAppCallback(() => {
     mockContextScope: async () => {
       throw new Error('mock create context failed');
     },
+    close: async () => {
+      // ...
+    },
+    backgroundTasksFinished: async () => {
+      // ...
+    },
   };
 });
 

diff --git a/test/fixtures/test-case-create-context-failed/test/index.test.js b/test/fixtures/test-case-create-context-failed/test/index.test.js
@@ -12,6 +12,12 @@ setGetAppCallback((suite, test) => {
         throw new Error('mock create context failed');
       }
     },
+    backgroundTasksFinished: async () => {
+      // ...
+    },
+    close: async () => {
+      // ...
+    },
   };
 });
 

diff --git a/test/fixtures/test-case-get-app-failed/test/index.test.js b/test/fixtures/test-case-get-app-failed/test/index.test.js
@@ -11,6 +11,12 @@ setGetAppCallback((suite, test) => {
     mockContextScope: async scope => {
       await scope({});
     },
+    backgroundTasksFinished: async () => {
+      // ...
+    },
+    close: async () => {
+      // ...
+    },
   };
 });
 

diff --git a/test/inject_ctx.test.js b/test/inject_ctx.test.js
@@ -64,6 +64,16 @@ describe('test/inject_ctx.test.js', () => {
   });
 
   describe('run suite', () => {
+    //   test/inject_ctx.test.js
+    //     run suite
+    //   1) "before all" hook: beforeAll in "{root}"
+    //   2) "after all" hook: afterAll in "{root}"
+    //   0 passing (7ms)
+    //   2 failing
+    //   1) "before all" hook: beforeAll in "{root}":
+    //      Error: mock get app failed
+    //   2) "after all" hook: afterAll in "{root}":
+    //      Error: mock get app failed
     it('get app error should failed', async () => {
       const fixture = path.join(__dirname, 'fixtures/get-app-failed');
 
@@ -78,13 +88,46 @@ describe('test/inject_ctx.test.js', () => {
       })
         // .debug()
         .expect('code', 1)
-        .expect('stdout', /get app for "root suite": mock get app failed/)
+        .expect('stdout', /"before all" hook: beforeAll in "{root}"/)
         .end();
     });
 
+    //   test/inject_ctx.test.js
+    //     run suite
+    //   1) "before all" hook: egg-mock-mock-ctx-failed in "{root}"
+    //   0 passing (4ms)
+    //   1 failing
+    //   1) "before all" hook: egg-mock-mock-ctx-failed in "{root}":
+    //      Error: mock create context failed
     it('create context error should failed', async () => {
       const fixture = path.join(__dirname, 'fixtures/create-context-failed');
 
+      await coffee.fork(eggBinFile, [
+        'test',
+        '-r', require.resolve('../register'),
+      ], {
+        cwd: fixture,
+        env: {
+          EGG_FRAMEWORK: require.resolve('egg'),
+        },
+      })
+        .debug()
+        .expect('code', 1)
+        .expect('stdout', /Error: mock create context failed/)
+        .end();
+    });
+
+    //   1) "before all" hook: beforeAll in "{root}"
+    //   2) "after all" hook: afterAll in "{root}"
+    //   0 passing (432ms)
+    //   2 failing
+    //   1) "before all" hook: beforeAll in "{root}":
+    //      Error: mock app ready failed
+    //   2) "after all" hook: afterAll in "{root}":
+    //      Error: mock app ready failed
+    it('app.ready error should failed', async () => {
+      const fixture = path.join(__dirname, 'fixtures/app-ready-failed');
+
       await coffee.fork(eggBinFile, [
         'test',
         '-r', require.resolve('../register'),
@@ -96,12 +139,19 @@ describe('test/inject_ctx.test.js', () => {
       })
         // .debug()
         .expect('code', 1)
-        .expect('stdout', /inject context for "root suite": mock create context failed/)
+        .expect('stdout', /mock app ready failed/)
         .end();
     });
   });
 
   describe('run test', () => {
+    //  test case get app error
+    //     1) should not print
+    //   0 passing (5ms)
+    //   1 failing
+    //   1) test case get app error
+    //        should not print:
+    //      Error: mock get app failed
     it('get app error should failed', async () => {
       const fixture = path.join(__dirname, 'fixtures/test-case-get-app-failed');
 
@@ -116,10 +166,19 @@ describe('test/inject_ctx.test.js', () => {
       })
         // .debug()
         .expect('code', 1)
-        .expect('stdout', /get app for "test case get app error - should not print": mock get app failed/)
+        .expect('stdout', /Error: mock get app failed/)
         .end();
     });
 
+    //   test case create context error
+    //     1) should not print
+    //   0 passing (7ms)
+    //   1 failing
+    //   1) test case create context error
+    //        should not print:
+    //      Error: mock create context failed
+    //       at Object.mockContextScope (test/index.test.js:12:15)
+    //       at next (/Users/killa/workspace/egg-mock/lib/inject_context.js:107:30)
     it('create context error should failed', async () => {
       const fixture = path.join(__dirname, 'fixtures/test-case-create-context-failed');
 
@@ -134,7 +193,7 @@ describe('test/inject_ctx.test.js', () => {
       })
         // .debug()
         .expect('code', 1)
-        .expect('stdout', /create context for "test case create context error" error: mock create context failed/)
+        .expect('stdout', /Error: mock create context failed/)
         .end();
     });
   });