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
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,30 @@ function _transformFakeAsyncCall(
`Transformed \`fakeAsync\` to \`vi.useFakeTimers\`.`,
);

let statements = callbackBody.statements;

// Append `vi.runOnlyPendingTimersAsync()` as the last statement of `beforeEach` to mimic flush behavior.
if (
ts.isCallExpression(node.parent) &&
ts.isIdentifier(node.parent.expression) &&
node.parent.expression.text === 'beforeEach' &&
!_isFakeAsyncFlushDisabled(node)
) {
statements = ts.factory.createNodeArray([
...statements,
ts.factory.createExpressionStatement(
ts.factory.createAwaitExpression(createViCallExpression(ctx, 'runOnlyPendingTimersAsync')),
),
]);
}

return ts.factory.createArrowFunction(
[ts.factory.createModifier(ts.SyntaxKind.AsyncKeyword)],
fakeAsyncCallback.typeParameters,
fakeAsyncCallback.parameters,
undefined,
ts.factory.createToken(ts.SyntaxKind.EqualsGreaterThanToken),
ts.factory.createBlock(callbackBody.statements),
ts.factory.createBlock(statements),
);
}

Expand Down Expand Up @@ -177,6 +194,25 @@ function _createFakeTimersHookStatements(ctx: RefactorContext): ts.Statement[] {
];
}

/**
* Detects if the `flush` option is set to false in the `fakeAsync` call expression.
* e.g. `fakeAsync(() => { ... }, { flush: false })`
*/
function _isFakeAsyncFlushDisabled(fakeAsyncCallExpression: ts.CallExpression): boolean {
const options = fakeAsyncCallExpression.arguments[1];

return (
options &&
ts.isObjectLiteralExpression(options) &&
options.properties.some(
(property) =>
ts.isPropertyAssignment(property) &&
property.name.getText() === 'flush' &&
property.initializer.getText() === 'false',
Comment thread
yjaaidi marked this conversation as resolved.
)
);
}

interface CurrentOutermostDescribeContext {
isUsingFakeAsync: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,28 +154,79 @@ describe('transformFakeAsyncTest', () => {
},
{
description:
'should transform fakeAsync test to `vi.useFakeTimers()` in `beforeEach`, `afterEach`, `beforeAll`, `afterAll`',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I split the different hooks in different tests as beforeEach diverges from others.

'should transform fakeAsync test to `vi.useFakeTimers()` in `beforeEach` and preserve flush behavior',
input: `
import { fakeAsync } from '@angular/core/testing';

describe('My fakeAsync suite', () => {
beforeAll(fakeAsync(() => {
console.log('beforeAll');

let count = 0;
beforeEach(fakeAsync(() => {
setTimeout(() => ++count, 100);
}));

afterAll(fakeAsync(() => {
console.log('afterAll');
it('works', fakeAsync(() => {
expect(count).toBe(1);
}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
});
afterEach(() => {
vi.useRealTimers();
});

beforeEach(fakeAsync(() => {
console.log('beforeEach');
}));
let count = 0;
beforeEach(async () => {
setTimeout(() => ++count, 100);
await vi.runOnlyPendingTimersAsync();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is what mimics best fakeAsync's flush option behavior.
I've only enabled it for beforeEach as it's the hook that is most likely to break tests if timers are not flushed.
I've decided not to apply it to tests and other hooks to reduce code pollution. On the other hand, it is also possible that some afterEach hooks make assertions that assume timers are flushed in tests.

While writing this, I realize that it would be more consistent to flush pending timers everywhere and not just in beforeEach hook.
@atscott @clydin what do you think?

});

it('works', async () => {
expect(count).toBe(1);
});
});
`,
},
{
description: 'should transform fakeAsync test to `vi.useFakeTimers()` in `afterEach`',
input: `
import { fakeAsync } from '@angular/core/testing';

describe('My fakeAsync suite', () => {
afterEach(fakeAsync(() => {
console.log('afterEach');
}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
});
afterEach(() => {
vi.useRealTimers();
});
afterEach(async () => {
console.log('afterEach');
});
});
`,
},
{
description: 'should transform fakeAsync test to `vi.useFakeTimers()` in `beforeAll`',
input: `
import { fakeAsync } from '@angular/core/testing';

describe('My fakeAsync suite', () => {
beforeAll(fakeAsync(() => {
console.log('beforeAll');
}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
Expand All @@ -187,17 +238,30 @@ describe('transformFakeAsyncTest', () => {
beforeAll(async () => {
console.log('beforeAll');
});
});
`,
},
{
description: 'should transform fakeAsync test to `vi.useFakeTimers()` in `afterAll`',
input: `
import { fakeAsync } from '@angular/core/testing';

afterAll(async () => {
describe('My fakeAsync suite', () => {
afterAll(fakeAsync(() => {
console.log('afterAll');
}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
});

beforeEach(async () => {
console.log('beforeEach');
afterEach(() => {
vi.useRealTimers();
});

afterEach(async () => {
console.log('afterEach');
afterAll(async () => {
console.log('afterAll');
});
});
`,
Expand Down Expand Up @@ -240,6 +304,65 @@ describe('transformFakeAsyncTest', () => {
});
`,
},
{
description: 'should not append `vi.runOnlyPendingTimersAsync()` in `test` or `afterEach`',
input: `
import { fakeAsync } from '@angular/core/testing';

describe('My fakeAsync suite', () => {
afterEach(fakeAsync(() => {
console.log('afterEach');
}));

it('works', fakeAsync(() => {
expect(1).toBe(1);
}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
});
afterEach(() => {
vi.useRealTimers();
});
afterEach(async () => {
console.log('afterEach');
});

it('works', async () => {
expect(1).toBe(1);
});
});
`,
},
{
description:
'should not append `vi.runOnlyPendingTimersAsync()` if `flush` option is set to false',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To keep the migration simple, we only detect object expression with false as a literal.

input: `
import { fakeAsync } from '@angular/core/testing';

describe('My fakeAsync suite', () => {
beforeEach(fakeAsync(() => {
console.log('beforeEach');
}, {flush: false}));
});
`,
expected: `
describe('My fakeAsync suite', () => {
beforeEach(() => {
vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
});
afterEach(() => {
vi.useRealTimers();
});
beforeEach(async () => {
console.log('beforeEach');
});
});
`,
},
];

testCases.forEach(({ description, input, expected }) => {
Expand Down
Loading