feat: add mock call history to access request configuration in test

[blephy]

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 getCallHistory class method to MockAgent instance
• add clearCallHistory class method to MockAgent instance
• add disableCallHistory class method to MockAgent instance
• add enableCallHistory class method to MockAgent instance
• add enableCallHistory MockAgent instantiation options
• clear all call history logs 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.clearCallHistory()
  })

  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.clearCallHistory()
  })

  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)
  })
})

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[metcoder95]

Can you add testing for it?

[blephy]

Yes for sur. I just need times

[mcollina]

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

[metcoder95]

lgtm, small comment left

[metcoder95]

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

[blephy]

@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 🙏🏼

[blephy]

@mcollina 😢

[mcollina]

Thanks for doing this.

I'm also sorry it took so long for me to check out. I've been buried in work for the last few weeks.

I have question: why is the call history log a singleton? I don't think it should be.

[blephy]

Hi @mcollina !

I saw that, don't worry about, i looked at your work and maintaining all of this open source projects + a job is not an easy part ;)

To answer at your question :

• MockCallHistoryLog is not a singleton. it contains information about one request configuration and is instantiable.
• MockCallHistory is an hybride singleton. it contains every mock call history log for a particular set of requests. You can instantiate a MockCallHistory but at instantiation the instance is registered on a private static class method to be able to :
  • clear every instante when needed
  • delete every instante when needed
  • retrieve an instance based on the name of the set of requests.

NB : the main purpose of the static class method is to avoid to register and associate an history instance to a MockAgent or a MockScope. In my own opinion, this is the most simple way to do it and the safer way to register multiple instance when constructing multiple MockAgent / MockScope withou having to forget to delete them.

[mcollina]

NB : the main purpose of the static class method is to avoid to register and associate an history instance to a MockAgent or a MockScope. In my own opinion, this is the most simple way to do it and the safer way to register multiple instance when constructing multiple MockAgent / MockScope withou having to forget to delete them.

Can you please remove it, and have it passed directly to MockAgent, or just create one in MockAgent if enabled? We already have one singleton (the default dispatcher), I don't want to have two to reset after each test.

[blephy]

Ok i will rollback to my previous implementation, but delete the ability to bind a mockCallHistory instance to a mockDispatch because without this static class method, i cannot get the mockCallHistory associated with a mockDispatch with mockAgent.getCallHistory(name) :

• attach a MockCallHistory instance to a MockAgent instance if / when enabled
• when mockAgent.close is called, the MockCallHistory instance associated with the mockAgent instance will be cleared (the option / instance will still be set for reuse)

WDYT @mcollina ?

ps : i can do it this night

[mcollina]

that should work

[blephy]

@mcollina this is done in this commit 0f6f62e

I also rebase on main and force pushed

[blephy]

@mcollina a little up :D

[mcollina]

lgtm

[blephy]

🎉 !

I let you merge guys

Thx for your time