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: implement mocking in feature tests #9715

Closed

Conversation

markw-deriv
Copy link
Contributor

@markw-deriv markw-deriv commented Aug 16, 2023

Refactor current end-to-end tests to work with mocks.

Local Mock

  • I've created an example exampleMock.ts file, that can be run with ts-node ./exampleMock.ts to start a web socket server on `localhost:10443'

Using mocks in tests

Each test should setup the mocks they need from the pool of mocks provided.

import { test, expect } from '@playwright/test';
import setupMocks from '../../utils/mocks/mocks';
import mockGeneral from '../../mocks/general';
import mockLoggedIn from '../../mocks/auth';

test('it shows the first name when logged in', async ({ page, baseURL }) => {
    await setupMocks({
        baseURL,
        page,
        mocks: [mockGeneral, mockLoggedIn],
    });
    await page.goto(`${baseURL}/app-store/traders-hub`);

    const firstName = await page.getByRole('button', { name: 'User Menu' }).first();
    expect(await firstName.inputValue()).toBeVisible()
});

Writing your own mocks

A mock is simply a function that is given a context object that contains the following keys:

  • req_id
  • request
  • response
export default function customTimeMock(context) {
    if (context.request.time === 1) {
        context.response = {
            echo_req: context.request,
            req_id: context.req_id,
            msg_type: 'time',
            time: (Date.now() / 1000).toFixed(0),
        };
    }
}

// Then add it to your mocks:
await setupMocks({
    baseURL,
    page,
    mocks: [mockGeneral, mockLoggedIn, customTimeMock],
});

Extending Mocks

If you want to change how a base mock works, or "extend" a mock, you can use the middleware like approach.

For example, let's say you want to add a day to the time endpoint. You can use the mockTime base mock, but add another mock to add the day key.

export default function mockTimeMonday(context) {
    if (context.request.time === 1) {
        context.response.day = 'Monday';
    }
}

await setupMocks({
    baseURL,
    page,
    mocks: [mockTime, mockTimeMonday],
});

| Note: The response will, by default, be undefined. Above, mockTime creates the first response, then mockTimeMonday get's the mutated response and adds a new key. For that reason, the order of mocks is important.

Dynamically changing

If you have a test that needs to change a mock, in the middle of the run, you can use the add and remove functions returned from createMockServer

const mocks = await setupMocks({
    baseURL,
    page,
    mocks: [mockGeneral, mockLoggedIn],
});

mocks.remove(mockLoggedIn)
mocks.add(mockHelloWorld)

@vercel
Copy link

vercel bot commented Aug 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Aug 17, 2023 4:00pm

@github-actions
Copy link
Contributor

github-actions bot commented Aug 16, 2023

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information.
- **PR**: [https://github.com/binary-com/deriv-app/pull/9715](https://github.com/binary-com/deriv-app/pull/9715)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-markwylde-deriv-integration-with-mocks.binary.sx?qa_server=red.binaryws.com&app_id=31580
    - **Original**: https://deriv-app-git-fork-markwylde-deriv-integration-with-mocks.binary.sx
- **App ID**: `31580`

@github-actions
Copy link
Contributor

github-actions bot commented Aug 16, 2023

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 21
🟧 Accessibility 75
🟢 Best practices 92
🟧 SEO 85
🟢 PWA 90

Lighthouse ran with https://deriv-app-git-fork-markwylde-deriv-integration-with-mocks.binary.sx/

@coveralls
Copy link

Coverage Status

coverage: 10.145%. remained the same when pulling e089a36 on markwylde-deriv:integration-with-mocks into aba9a9f on binary-com:master.

@ali-hosseini-deriv
Copy link
Member

@markwylde-deriv nice job. i have some questions and concerns here:

  1. How we can handle features specific to special regions? do we have to create another mock?
  2. Let's say we want to test something that is specific to Demo account, in that case do we have to manually switch to demo? or need to do it using mocks?
  3. Let's say as a developer I want to have some specific response from BE, in that case i need to create another mock just like the one that we have here. my concern here is that wouldn't it be so much duplicated code(mock)? in other words is there any way that we can do some abstractions in the mock data?

@farzin-deriv
Copy link
Contributor

@markwylde-deriv In the above example, We only set the mocks when starting the test, How to change the mock in the flow? 🤔

get_setting = foo
user saves the form with set_settings => bar
expect get_setting = bar

@markw-deriv
Copy link
Contributor Author

markw-deriv commented Aug 16, 2023

@markwylde-deriv In the above example, We only set the mocks when starting the test, How to change the mock in the flow? 🤔

get_setting = foo
user saves the form with set_settings => bar
expect get_setting = bar

Hi @farzin-deriv

Great point. I haven't catered for this. Would something like this work?

    test('it shows the current name', async ({ page, baseURL }) => {
        const mocks = await setupMocks({
            baseURL,
            page,
            mocks: [mockGeneral, mockLoggedIn, mockResidentsList, mockStatesList],
        });
        await page.goto(`${baseURL}/account/personal-details`);

        const firstName = await page.getByLabel('First name*').first();
        expect(await firstName.inputValue()).toBe('Jane');

        const lastName = await page.getByLabel('Last name*').first();
        expect(await lastName.inputValue()).toBe('Smith');

        mocks.add(mockSecondThing);

        const dateOfBirth = await page.getByLabel('Date of birth*').first();
        expect(await dateOfBirth.inputValue()).toBe('01-01-1980');

        mocks.remove(mockSecondThing);

        const citizenship = await page.getByLabel('Citizenship').first();
        expect(await citizenship.inputValue()).toBe('Thailand');
    });

@markw-deriv
Copy link
Contributor Author

@markwylde-deriv nice job. i have some questions and concerns here:

  1. How we can handle features specific to special regions? do we have to create another mock?

I think there's a couple of ways to do this.

Extending

We can do this as it follows a sort of middleware/chain flow. So just a mock after the first one, then change the response how you want.

function add_req_id ({ request, response, req_id }) {
  response = {
    req_id
  }
}

function add_hello ({ request, response, req_id }) {
  response = {
    hello: 'world'
  }
}

await setupMocks({
        baseURL,
        page,
        mocks: [add_req_id, add_hello],
});

Override

A mock could also be given arguments. For example:

const mock_time = (override) => (context) {
    if (context.request.time === 1) {
        context.response = {
            echo_req: context.request,
            req_id: context.req_id,
            msg_type: 'time',
            time: (Date.now() / 1000).toFixed(0),
            ...override
        };
    }
}

await setupMocks({
        baseURL,
        page,
        mocks: [mock_time({ hello: 'world' })],
});
  1. Let's say we want to test something that is specific to Demo account, in that case do we have to manually switch to demo? or need to do it using mocks?

I'd say use mocks. For anything that needs an actual server, I think the end-to-end/qa team should focus on that. So yeah, we would mock the demo account. However, you could just extend from mock (as proposed above?)

  1. Let's say as a developer I want to have some specific response from BE, in that case i need to create another mock just like the one that we have here. my concern here is that wouldn't it be so much duplicated code(mock)? in other words is there any way that we can do some abstractions in the mock data?

Does the first point answer this?

@farzin-deriv
Copy link
Contributor

Hi @farzin-deriv

Great point. I haven't catered for this. Would something like this work?

    test('it shows the current name', async ({ page, baseURL }) => {
        const mocks = await setupMocks({
            baseURL,
            page,
            mocks: [mockGeneral, mockLoggedIn, mockResidentsList, mockStatesList],
        });
        await page.goto(`${baseURL}/account/personal-details`);

        const firstName = await page.getByLabel('First name*').first();
        expect(await firstName.inputValue()).toBe('Jane');

        const lastName = await page.getByLabel('Last name*').first();
        expect(await lastName.inputValue()).toBe('Smith');

        mocks.add(mockSecondThing);

        const dateOfBirth = await page.getByLabel('Date of birth*').first();
        expect(await dateOfBirth.inputValue()).toBe('01-01-1980');

        mocks.remove(mockSecondThing);

        const citizenship = await page.getByLabel('Citizenship').first();
        expect(await citizenship.inputValue()).toBe('Thailand');
    });

I think we should be able to override the default mocks, instead of replacing the whole thing 🤔 The API could look something like this maybe;

// Set up test stuff
const server = createServer(...);

// Perform an action and return mocked response
server.send(...).ressponse(...);

// Do assertion
expect(stuff).toBe(goodStuff);

@markw-deriv
Copy link
Contributor Author

Yeah @farzin-deriv. I think you're right. Would my response to Ako solve this?
#9715 (comment)

@farzin-deriv
Copy link
Contributor

If I remember correctly @mohsen-deriv did some good stuff before for mocking the server and providing the tooling for the test, Why abandon that POC? 😅

@markw-deriv
Copy link
Contributor Author

If I remember correctly @mohsen-deriv did some good stuff before for mocking the server and providing the tooling for the test, Why abandon that POC? 😅

Yeah, Michio's stuff looked cool, specifically around the types/typescript of the websocket endpoint. Could come in really handy. I'll speak to him about how we can implement that in here.

@markw-deriv
Copy link
Contributor Author

In bdb5045, createMockServer will now return add and remove, allowing you to dynamically adjust mocks in the middle of a test.

@sonarcloud
Copy link

sonarcloud bot commented Aug 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

No Coverage information No Coverage information
53.6% 53.6% Duplication

@niloofar-deriv
Copy link
Contributor

@markwylde-deriv Thanks for your complete documentation 🙏 🔥

import { Context } from '../../utils/mocks/mocks';

export default function mock_landing_company(context: Context) {
if ('landing_company' in context.request && context.request.landing_company === 'th') {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have ts error here 🤔

image

deposit_commission: '0',
email: 'teclasarojani@gmail.com',
further_information:
'ආයුබෝවන් ඔබට අවශ්‍ය Binary / Deriv $ මිලට ගැනීමට අප සමග සමිබන්ධ වෙන්න. Bank Transfer( cash ), USDT Exchange , E-wallet Exchange සමග ඉතා කඩිනම් සුහදශීලී හා ලාබදායි ලෙස ඔබට අපගෙන් ලබාගත හැක. Binary/ Deriv Dollars,USDT, Crypto மாற்றம் அல்லது வாங்குதல் மற்றும் விற்பனை நோக்கங்களுக்காக என்னை தொடர்பு கொள்ளவும். குறைந்த விகிதங்கள் மற்றும் வேகமான சேவைகள் எங்களிடம் பெற்றுக்கொள்ளலாம். You are warmly welcome to my premium payment agent group. Whatsapp following Number for more details.',
Copy link
Contributor

Choose a reason for hiding this comment

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

What language is this ? 😅

if (
'platform' in context.request &&
context.request.platform === 'dxtrade' &&
context.request.trading_servers === 1
Copy link
Contributor

Choose a reason for hiding this comment

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

We have ts error here
image

import { Context } from '../../utils/mocks/mocks';

export default function mock_platform_mt5(context: Context) {
if ('platform' in context.request && context.request.platform === 'mt5' && context.request.trading_servers === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

image

if (
'trading_platform_accounts' in context.request &&
context.request.trading_platform_accounts === 1 &&
context.request.platform === 'derivez'
Copy link
Contributor

Choose a reason for hiding this comment

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

image

import mock_time from './time';
import mock_website_status from './website_status';

const general = context => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const general = context => {
const general = (context: Context) => {

Can we define type for this? 🤔

import { Context } from '../../utils/mocks/mocks';

function mock_residents_list(context: Context) {
if (context.request.residence_list === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

image

import { Context } from '../../utils/mocks/mocks';

function mock_states_list(context: Context) {
if (context.request.states_list === 'th') {
Copy link
Contributor

Choose a reason for hiding this comment

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

image


wss.on('connection', ws => {
ws.on('message', async message => {
const parsedMessage = JSON.parse(message);
Copy link
Contributor

Choose a reason for hiding this comment

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

image

@@ -0,0 +1,70 @@
import { Context } from '../../utils/mocks/mocks';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use absolute import instead of this pattern? 🤔

Copy link

@review-deriv review-deriv left a comment

Choose a reason for hiding this comment

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

Pull Request Too Large

This pull request is too large, and reviewing it will be difficult. In addition, because of ChatGPT API limitations, this PR cannot be sent for external review. Please consider breaking this PR into smaller ones

Copy link

@review-deriv review-deriv left a comment

Choose a reason for hiding this comment

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

Pull Request Too Large

This pull request is too large, and reviewing it will be difficult. In addition, because of ChatGPT API limitations, this PR cannot be sent for external review. Please consider breaking this PR into smaller ones

@markw-deriv
Copy link
Contributor Author

POC moved to #9761

@markw-deriv markw-deriv closed this Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants