From 4374931b0ed331450c77ca19cf90a5c0c39b2230 Mon Sep 17 00:00:00 2001
From: JiaLiPassion <JiaLi.Passion@gmail.com>
Date: Thu, 12 Mar 2020 01:41:19 +0900
Subject: [PATCH] fix(zone.js): zone.js patch jest should handle done correctly
 (#36022)

`zone.js` supports jest `test.each()` methods, but it
introduces a bug, which is the `done()` function will not be handled correctly.

```
it('should work with done', done => {
  // done will be undefined.
});
```

The reason is the logic of monkey patching `test` method is different from `jasmine` patch

// jasmine patch
```
return testBody.length === 0
   ? () => testProxyZone.run(testBody, null)
   : done => testProxyZone.run(testBody, null, [done]);
```

// jest patch
```
 return function(...args) {
   return testProxyZone.run(testBody, null, args);
 };
```

the purpose of this change is to handle the following cases.

```
test.each([1, 2])('test.each', (arg1, arg2) => {
  expect(arg1).toBe(1);
  expect(arg2).toBe(2);
});
```

so in jest, it is a little complex, because the `testBody`'s parameter may be bigger than 1, so the
logic in `jasmine`

```
return testBody.length === 0
   ? () => testProxyZone.run(testBody, null)
   : done => testProxyZone.run(testBody, null, [done]);
```
will not work for `test.each` in jest.

So in this PR, I created a dynamic `Function` to return the correct length of paramters (which is required by jest core), to handle
1. normal `test` with or without `done`.
2. each with parameters with or without done.

PR Close #36022
---
 packages/zone.js/lib/jest/jest.ts       | 18 ++++--
 packages/zone.js/test/jest/jest.spec.js | 85 +++++++++++++++++++++----
 2 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/packages/zone.js/lib/jest/jest.ts b/packages/zone.js/lib/jest/jest.ts
index 28e989c0d4..7ced7e9a78 100644
--- a/packages/zone.js/lib/jest/jest.ts
+++ b/packages/zone.js/lib/jest/jest.ts
@@ -43,10 +43,9 @@ Zone.__load_patch('jest', (context: any, Zone: ZoneType) => {
 
   function wrapTestFactoryInZone(originalJestFn: Function) {
     return function(this: unknown, ...tableArgs: any[]) {
-      const testFn = originalJestFn.apply(this, tableArgs);
       return function(this: unknown, ...args: any[]) {
         args[1] = wrapTestInZone(args[1]);
-        return testFn.apply(this, args);
+        return originalJestFn.apply(this, tableArgs).apply(this, args);
       };
     };
   }
@@ -64,16 +63,21 @@ Zone.__load_patch('jest', (context: any, Zone: ZoneType) => {
   /**
    * Gets a function wrapping the body of a jest `it/beforeEach/afterEach` block to
    * execute in a ProxyZone zone.
-   * This will run in the `testProxyZone`.
+   * This will run in the `proxyZone`.
    */
   function wrapTestInZone(testBody: Function): Function {
     if (typeof testBody !== 'function') {
       return testBody;
     }
-    // The `done` callback is only passed through if the function expects at least one argument.
-    // Note we have to make a function with correct number of arguments, otherwise jest will
-    // think that all functions are sync or async.
-    return function(this: unknown, ...args: any[]) { return proxyZone.run(testBody, this, args); };
+    const wrappedFunc = function() {
+      return proxyZone.run(testBody, null, arguments as any);
+    };
+    // Update the length of wrappedFunc to be the same as the length of the testBody
+    // So jest core can handle whether the test function has `done()` or not correctly
+    Object.defineProperty(
+        wrappedFunc, 'length', {configurable: true, writable: true, enumerable: false});
+    wrappedFunc.length = testBody.length;
+    return wrappedFunc;
   }
 
   ['describe', 'xdescribe', 'fdescribe'].forEach(methodName => {
diff --git a/packages/zone.js/test/jest/jest.spec.js b/packages/zone.js/test/jest/jest.spec.js
index 13e3841279..35fa8df264 100644
--- a/packages/zone.js/test/jest/jest.spec.js
+++ b/packages/zone.js/test/jest/jest.spec.js
@@ -6,10 +6,18 @@ function assertInsideSyncDescribeZone() {
 }
 describe('describe', () => {
   assertInsideSyncDescribeZone();
-  beforeEach(() => { assertInsideProxyZone(); });
-  beforeAll(() => { assertInsideProxyZone(); });
-  afterEach(() => { assertInsideProxyZone(); });
-  afterAll(() => { assertInsideProxyZone(); });
+  beforeEach(() => {
+    assertInsideProxyZone();
+  });
+  beforeAll(() => {
+    assertInsideProxyZone();
+  });
+  afterEach(() => {
+    assertInsideProxyZone();
+  });
+  afterAll(() => {
+    assertInsideProxyZone();
+  });
 });
 describe.each([[1, 2]])('describe.each', (arg1, arg2) => {
   assertInsideSyncDescribeZone();
@@ -17,23 +25,78 @@ describe.each([[1, 2]])('describe.each', (arg1, arg2) => {
   expect(arg2).toBe(2);
 });
 describe('test', () => {
-  it('it', () => { assertInsideProxyZone(); });
+  it('it', () => {
+    assertInsideProxyZone();
+  });
   it.each([[1, 2]])('it.each', (arg1, arg2) => {
     assertInsideProxyZone();
     expect(arg1).toBe(1);
     expect(arg2).toBe(2);
   });
-  test('test', () => { assertInsideProxyZone(); });
-  test.each([[]])('test.each', () => { assertInsideProxyZone(); });
+  test('test', () => {
+    assertInsideProxyZone();
+  });
+  test.each([[]])('test.each', () => {
+    assertInsideProxyZone();
+  });
 });
 
-it('it', () => { assertInsideProxyZone(); });
-it.each([[1, 2]])('it.each', (arg1, arg2) => {
+it('it', () => {
+  assertInsideProxyZone();
+});
+it('it with done', done => {
+  assertInsideProxyZone();
+  done();
+});
+
+it.each([[1, 2]])('it.each', (arg1, arg2, done) => {
   assertInsideProxyZone();
   expect(arg1).toBe(1);
   expect(arg2).toBe(2);
+  done();
+});
+
+it.each([2])('it.each with 1D array', arg1 => {
+  assertInsideProxyZone();
+  expect(arg1).toBe(2);
+});
+
+it.each([2])('it.each with 1D array and done', (arg1, done) => {
+  assertInsideProxyZone();
+  expect(arg1).toBe(2);
+  done();
+});
+
+it.each`
+    foo  | bar
+    ${1} | ${2}
+  `('it.each should work with table as a tagged template literal', ({foo, bar}) => {
+  expect(foo).toBe(1);
+  expect(bar).toBe(2);
+});
+
+it.each`
+    foo  | bar
+    ${1} | ${2}
+  `('it.each should work with table as a tagged template literal with done', ({foo, bar}, done) => {
+  expect(foo).toBe(1);
+  expect(bar).toBe(2);
+  done();
+});
+
+it.each`
+    foo  | bar
+    ${1} | ${2}
+  `('(async) it.each should work with table as a tagged template literal', async ({foo, bar}) => {
+  expect(foo).toBe(1);
+  expect(bar).toBe(2);
+});
+
+test('test', () => {
+  assertInsideProxyZone();
+});
+test.each([[]])('test.each', () => {
+  assertInsideProxyZone();
 });
-test('test', () => { assertInsideProxyZone(); });
-test.each([[]])('test.each', () => { assertInsideProxyZone(); });
 
 test.todo('todo');