Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(component-testing): implement mocks #1027

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DudaGod
Copy link
Member

@DudaGod DudaGod commented Oct 25, 2024

What is done:

Implement ability to mock modules and use spies in component testing. The same as in jest.

How it works

I'll explain it with a simple example:

// tests/test.testplane.tsx
import { render } from '@testing-library/react';
import App from '../App';
// testplane/mock - new file which is exports methods from `@vitest/spy` and custom methods: `mock`, `unmock`. 
// These methods works only in browser env;
import {fn, mock} from "testplane/mock"; 
import { handleClick } from './utils';

mock('./utils', () => ({
    handleClick: fn().mockImplementationOnce(() => {
        console.log('Handle click by mock');
    })
}));

it('should render react button', async ({browser}) => {
    render(
        <App />
    );

    await browser.$('input').click();

    expect(handleClick).toHaveBeenCalledTimes(1);
});
// App.tsx
import {handleClick} from "./tests/utils";

export default function App() {
    return (
        <div id="root">
            <input
                type="button"
                value="Some button"
                onClick={handleClick}
            />
        </div >
    );
}
// utils.ts
export const handleClick = (): void => {
    console.log('Handle click by user code');
}

In order to correctly mock handleClick method from utils.ts source code of test file and dependencies which used mocked module modified in runtime. Test file will looks like:

// tests/test.testplane.tsx
import * as __testplane_import_1__ from './utils';
import {fn, mock} from "testplane/mock";

await mock('./utils', () => ({
    handleClick: fn().mockImplementationOnce(() => {
        console.log('Handle click by mock');
    })
}), __testplane_import_1__);

const { render } = await import('@testing-library/react');
const App = await import('../App');
const { handleClick } = importWithMock('./utils', __testplane_import_1__);

// ...

What happens:

  • import of original module utils moved to the top and saves to generated variable - __testplane_import_1__. It is used in order to correctly mock module (for example if user mock only on method);
  • import of fn and mock also moved to the top in order to call mock before import all other modules;
  • call mock with await and send original module as third argument. Moreover move it to the top. Here used mock from src/runner/browser-env/vite/browser-modules/mock.ts which is async.
  • import all other modules using dynamic imports. It is necessary in order to correctly mock module and then import all other deps in which mocked module can be imported. Without dynamic import modules will imported first and then await mock will be executed;
  • for mocked module use helper importWithMock which gets mocked implementation of module instead of original module.

Component file will looks like (after modify in runtime):

import * as __testplane_import_2__ from './tests/utils';
const {handleClick} = importWithMock("./tests/utils", __testplane_import_2__);

export default function App() {
    // ...
}

What happens:

  • like in test file we save original module to special variable - __testplane_import_2__;
  • import mocked module using helper importWithMock

Discuss:

  • Should rename new exported file mock for something more understandable? This file not only export mock and unmock methods but also spied: fn, spyOn. Moreover this module works only in browser environment. And can't be called in node env.

@DudaGod DudaGod force-pushed the TESTPLANE-275.mocks branch 2 times, most recently from c7a350c to 2c471d4 Compare October 25, 2024 09:20
package.json Outdated
@@ -34,6 +33,10 @@
"type": "git",
"url": "git://github.com/gemini-testing/testplane.git"
},
"exports": {
".": "./src/index.js",
"./mock": "./src/mock.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

Can be used correctly only in component testing, where this module replaced only in browser env in order to correctly mock modules. In node env provide only stubs of mock and unmock in order to be able to read tests.

Other options that I've been considering:

  • export mock interface from the main file. Can be implemented but in this case in browser env I will not be able to use something else from main module, because it is replaced on browser implementation;
  • create another package and replace it in browser env. Can be done, but I don't sure that we need another small package with stub mock, unmock and reexport vite/spy

Problem with current solution:

  • user can import this module - testplane/mock in nodejs environment and try to use it (I don't know why and looks like that in integration tests it should not be used). But it will not works correctly because mock and unmock has stubs right now and replaced on browser env. I can fix it by throw error and ignore if user run in browser env. Not implemented right now, but I can do it;

What do you think?

package.json Outdated
@@ -56,6 +59,7 @@
"@jspm/core": "2.0.1",
"@types/debug": "4.1.12",
"@types/yallist": "4.0.4",
"@vitest/spy": "2.1.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

Used in order to reexport fn and spyOn from testplane/mock

@@ -81,6 +85,7 @@
"mocha": "10.2.0",
"plugins-loader": "1.3.4",
"png-validator": "1.1.0",
"recast": "0.23.6",
Copy link
Member Author

Choose a reason for hiding this comment

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

Used in browser env in order to correctly modify ast tree to correctly work with mock

src/mock.ts Outdated
export function mock(_moduleName: string, _factory?: MockFactory): void {}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export function unmock(_moduleName: string): void {}
Copy link
Member Author

Choose a reason for hiding this comment

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

Here mock and unmock do nothing (stubs). This module replaced in browser env using vite plugin.

@@ -0,0 +1,41 @@
export * from "@vitest/spy";
Copy link
Member Author

Choose a reason for hiding this comment

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

This module replace testplane/mock and works only in browser environment.

Copy link
Member

Choose a reason for hiding this comment

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

This module replaces testplane/mock and works only in browser environment.

Lets leave a comment about it at the start of the file

throw new Error(`Cannot find mocked module "${mockModuleName}"`);
}

return b.expressionStatement(b.awaitExpression(mockCallExpression));
Copy link
Member Author

Choose a reason for hiding this comment

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

Here code like:

mock("./utils", () => {...});

Will be modifed to code like:

await mock("./utils", () => {...}, __testplane_import_1__);

Here:

  • await is used because browser implementation of mock is async
  • __testplane_import_1__ - the name of original module, which is passed to mock implementation in file src/runner/browser-env/vite/browser-modules/mock.ts. Used in order to correctly handle factory.

return b.expressionStatement(b.awaitExpression(mockCallExpression));
});

ast.program.body.unshift(...preparedMockCalls);
Copy link
Member Author

Choose a reason for hiding this comment

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

Add new lines of code to the top of the file.
In order to run code with mocking module before code with import method, which is mocked.

It gives ability to user first import method and then mock it.


// Move import module with mocks to the top of the file
state.mockCalls.push(declaration);
nodePath.prune();
Copy link
Member Author

Choose a reason for hiding this comment

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

here remove original nodePath. It will be move to the top from state.mockCalls at the end.

const mockCall = exp.expression as types.namedTypes.CallExpression;

if (mockCall.arguments.length === 1) {
manualMock.mock((mockCall.arguments[0] as types.namedTypes.StringLiteral).value);
Copy link
Member Author

Choose a reason for hiding this comment

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

Use manual mock if user do not specify factory. It means write mock like this - mock("axios")

manualMock.mock((mockCall.arguments[0] as types.namedTypes.StringLiteral).value);
} else {
if ((exp.expression as types.namedTypes.CallExpression).arguments.length) {
registeredMocks.add(
Copy link
Member Author

Choose a reason for hiding this comment

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

Save mocks with factory which must be executed with original module. It means like this:

mock('./utils', () => ({
  handleClick: fn()
}));

".": "./build/src/index.js",
"./mock": "./build/src/mock/index.js"
},
"typesVersions": {
Copy link
Member Author

Choose a reason for hiding this comment

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

In order to correctly set paths to types

@@ -0,0 +1,580 @@
// TODO: use @vitest/spy when migrate to esm
Copy link
Member Author

Choose a reason for hiding this comment

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

This file copied as is from @vitest/spy and used in nodejs environment. In browser environment this file replaced to special file which works only in browser env.

Copied because module is esm and I can't use it correctly in cjs world.

Copy link
Member

Choose a reason for hiding this comment

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

This file copied as is from @vitest/spy

Lets leave a comment about it

Copy link
Member

@KuznetsovRoman KuznetsovRoman left a comment

Choose a reason for hiding this comment

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

Other than exports in package.json and unreliable isRunInBrowserEnv check, /ok
Also checking "absModuleUrl" with absolute module name (starting with /) is preferred

Comment on lines +36 to +39
"exports": {
".": "./build/src/index.js",
"./mock": "./build/src/mock/index.js"
},
Copy link
Member

@KuznetsovRoman KuznetsovRoman Nov 13, 2024

Choose a reason for hiding this comment

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

After that, people wouldn't have ability to import from somewhere else
Maybe we could get rid of it, return "main" and export "mock" from "testplane" so we could do this:

import { mock } from "testplane"

or something like that.

I think we shouldn't define exports section in package.json until testplane@9

@@ -0,0 +1,580 @@
// TODO: use @vitest/spy when migrate to esm
Copy link
Member

Choose a reason for hiding this comment

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

This file copied as is from @vitest/spy

Lets leave a comment about it

@@ -0,0 +1,41 @@
export * from "@vitest/spy";
Copy link
Member

Choose a reason for hiding this comment

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

This module replaces testplane/mock and works only in browser environment.

Lets leave a comment about it at the start of the file

}

export async function mock(moduleName: string, factory?: MockFactory, originalImport?: unknown): Promise<void> {
// Mock call without factory parameter is handled by manual-mock module and removed from the source code by mock vite plugin
Copy link
Member

Choose a reason for hiding this comment

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

is handled

So it should be

and is being removed

}

const { file, mockCache } = window.__testplane__;
const isModuleLocal = moduleName.startsWith("/") || moduleName.startsWith("./") || moduleName.startsWith("../");
Copy link
Member

Choose a reason for hiding this comment

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

Are custom paths supported in component testing?
If yes, then will it work with local paths like "src/foo/bar"?

@@ -151,6 +138,14 @@ function generateTemplate(env: WorkerInitializePayload, runUuid: string): string
<link rel="icon" href="https://testplane.io/img/favicon.ico">
<script type="module">
window.__testplane__ = ${JSON.stringify({ runUuid, ...env })};
window.__testplane__.mockCache = new Map();
window.importWithMock = function(modName, mod) {
if (window.__testplane__.mockCache.get(modName)) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Why .get and hot .has here?

window.__testplane__.mockCache = new Map();
window.importWithMock = function(modName, mod) {
if (window.__testplane__.mockCache.get(modName)) {
return window.__testplane__.mockCache.get(modName)
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing semicolumn

Comment on lines +44 to +46
if (!req.url?.endsWith("index.html") || !req.originalUrl) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

How it behaves with query("?") or hash location("#") parameters?
And what is the difference between "url" and "originalUrl"?

return fsPath;
}

return fsPath.slice(0, extname.length * -1);
Copy link
Member

Choose a reason for hiding this comment

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

Why extname.length * -1 and not -extname.length? extname.length * -1 looks weird

Comment on lines +9 to +11
export const isRunInBrowserEnv = (): boolean => {
return typeof window !== "undefined" && typeof window.document !== "undefined";
};
Copy link
Member

Choose a reason for hiding this comment

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

What if i mocked "globalThis.window" in node.js environment?
There are more reliable checks. I like this one, with global and window equality to this (i think, we better should compare with globalThis): https://stackoverflow.com/a/31090240

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants