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: add mock call history to access request configuration in test #4029

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

blephy
Copy link

@blephy blephy commented Jan 25, 2025

This relates to...

#4025

Rationale

Actually we are not able with MockAgent to assert request configuration.

Given this in mind, the only way we could do it is by wrapping our app http call in function, and then spy or mock that function with our favorite test framework. We can make assertions on parameters we passed through that function but, even with this possibility, we are not certain that undici did not make computation on inputs.

This PR is addressing this problematic : ensuring http calls are made with the expected low level configuration, as far as we can.

Changes

  • add MockCallHistory and MockCallHistoryLog class
  • add registerCallHistory class method to MockScope instance
  • add clearAllCallHistory class method to MockAgent instance
  • add disableCallHistory class method to MockAgent instance
  • add enableCallHistory class method to MockAgent instance
  • add enableCallHistory MockAgent instantiation options
  • delete all call history instances when MockAgent.close is called

Features

Given this application :

function myCall(body: Record<string, unknown>, token: string) {
  await fetch(`https://my-service.io/endpoint`, {
      method: 'POST',
      body: JSON.stringify(body),
      headers: {
        authorization: 'Bearer ${token}',
        'content-type': 'application/json'
      }
  })
}

async function startApp(): Promise<void> {
  await myCall({ data: 'hello' }, 'myToken')
}

After :

describe('my app', () => {
  let agent: MockAgent

  before(() => {
    agent = new MockAgent({ enableCallHistory: true })
    setGlobalDispatcher(agent)
  })

  beforeEach(() => {
    agent.clearAllCallHistory()
  })

  after(async () => {
    await agent.close()
  })

  it('should request my external service with a specific body and headers', async () => {
    mockAgent.get('https://my-service.io').intercept({ path: '/endpoint', method: 'POST' }).reply(201, 'OK').times(1)

    await startApp()

    expect(mockAgent.getCallHistory()?.firstCall()?.body).toStrictEqual('"{ "data": "hello" }"')
    expect(mockAgent.getCallHistory()?.firstCall()?.headers['authorization']).toStrictEqual('Bearer myToken')
    expect(mockAgent.getCallHistory()?.firstCall()?.headers['content-type']).toStrictEqual('application/json')
    expect(mockAgent.getCallHistory()?.firstCall()?.path).toStrictEqual('/endpoint')
    expect(mockAgent.getCallHistory()?.firstCall()?.protocol).toStrictEqual('https:')
    expect(mockAgent.getCallHistory()?.calls()?.length).toStrictEqual(1)
  })

  it('should always call external services with a secure HTTP protocol', async () => {
    await startApp()

    // working without intercepting too

    expect(mockAgent.getCallHistory()?.filterCalls({ protocol: /^((?!https).)*$/ })?.length).toStrictEqual(0)
  })
})

Before :

describe('my app', () => {
  let agent: MockAgent

  before(() => {
    agent = new MockAgent({ enableCallHistory: true })
    setGlobalDispatcher(agent)
  })

  beforeEach(() => {
    agent.clearAllCallHistory()
  })

  after(async () => {
    await agent.close()
  })

  it('should request my external service with a specific body and headers', async () => {
    const spy = spyOn(myCall)
    mockAgent.get('https://my-service.io').intercept({ path: '/endpoint', method: 'POST' }).reply(201, 'OK').times(1)

    await startApp()

    expect(spy.calls?.[0]).toHaveBeenCalledWith({ data: 'hello' }, 'myToken')
    // we cannot do it, we must modify our application code and this will only assert function parameters, not the real http request
    // expect(mockAgent.getCallHistory()?.firstCall()?.headers['authorization']).toMatch(/^Bearer /)
    // we cannot do it, we must modify our application code and this will only assert function parameters, not the real http request
    // expect(mockAgent.getCallHistory()?.firstCall()?.headers['content-type']).toStrictEqual('application/json')
    // we cannot do it, we must modify our application code and this will only assert function parameters, not the real http request
    // expect(mockAgent.getCallHistory()?.firstCall()?.path).toStrictEqual('/endpoint')
    // we cannot do it, we must modify our application code and this will only assert function parameters, not the real http request
    // expect(mockAgent.getCallHistory()?.firstCall()?.protocol).toStrictEqual('https:')
    expect(spy.calls.length).toStrictEqual(1)
  })

  it('should always call external services with a secure HTTP protocol', async () => {
    await startApp()

    // we cannot do it, we must modify our application code and this will only assert function parameters, not the real http request
    // expect(mockAgent.getCallHistory()?.filterCalls({ protocol: /^((?!https).)*$/ })?.length).toStrictEqual(0)
  })
})

Bug Fixes

Breaking Changes and Deprecations

Status

@metcoder95 metcoder95 linked an issue Jan 26, 2025 that may be closed by this pull request
Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

Can you add testing for it?

lib/mock/mock-interceptor.js Outdated Show resolved Hide resolved
@blephy
Copy link
Author

blephy commented Jan 26, 2025

Yes for sur. I just need times

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR! Can you please add a unit test?

lib/mock/mock-call-history.js Outdated Show resolved Hide resolved
@blephy blephy requested review from mcollina and metcoder95 January 27, 2025 00:28
@blephy blephy changed the title feat: add mock call histories feat: add mock call history Jan 27, 2025
@blephy blephy changed the title feat: add mock call history feat: add mock call history to access request configuration in test Jan 27, 2025
types/index.d.ts Outdated Show resolved Hide resolved
docs/docs/best-practices/mocking-request.md Outdated Show resolved Hide resolved
lib/mock/mock-call-history.js Outdated Show resolved Hide resolved
lib/mock/mock-interceptor.js Outdated Show resolved Hide resolved
test/mock-agent.js Outdated Show resolved Hide resolved
test/mock-agent.js Show resolved Hide resolved
test/mock-agent.js Fixed Show fixed Hide fixed
test/mock-agent.js Dismissed Show dismissed Hide dismissed
test/mock-agent.js Fixed Show fixed Hide fixed
test/mock-agent.js Fixed Show fixed Hide fixed
@blephy blephy requested a review from metcoder95 January 28, 2025 19:26
Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

lgtm, small comment left

lib/mock/mock-call-history.js Show resolved Hide resolved
@blephy blephy requested a review from metcoder95 January 29, 2025 18:20
Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

LGTM, left minor comment and should be ready on my end

lib/mock/mock-agent.js Outdated Show resolved Hide resolved
@blephy blephy requested a review from metcoder95 January 30, 2025 11:27
@blephy
Copy link
Author

blephy commented Jan 31, 2025

@mcollina can you take a bit of time to review back this PR ? it would be nice to have this change in the next release 🙏🏼

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.

Mock history
3 participants