[NP] add HTTP resources testing strategies

[mshustov]

Summary

Part of #53135
Documents integration- & unit-testing strategies for HTTP resources

[skip-ci]

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[pgayvallet]

This is a very good presentation of existing testing tools, their pros/cons, and their preferred usage, with associated route testing exemples.

I'm just wondering if the type of tests presentation should really belongs in the HTTP Routes subsection or we should add a higher level section with them. Else I'm afraid we are going to introduce a lot of redundancy / repetition when describing how to test the other functional sections (so/es...), as the type of tests are quite similar.

Some rewordings/rephrasing comments: (please ignore some or all)

[mshustov]

I'm just wondering if the type of tests presentation should really belongs in the HTTP Routes subsection or we should add a higher level section with them. Else I'm afraid we are going to introduce a lot of redundancy / repetition when describing how to test the other functional sections (so/es...), as the type of tests are quite similar.

It depends on the content of the other sections. 🤔
Do you want me to place them under Tools sections, for example?

[pgayvallet]

It depends on the content of the other sections. 🤔
Do you want me to place them under Tools sections, for example?

I have the feeling the other sections will have a quite similar layout regarding the type of tests, so I think having a 'tool presentation' section before the guidelines on how to test the different part of the plugins would make sense yes.

Would like to have the others' opinion before having you restructure all this though. @rudolf @joshdover @eliperelman WDYT ?

[mshustov]

@rudolf does it answer your question about route controllers testing #51150? I think the part with unit-tests should do the trick. Let me know if I can improve the docs.

[pgayvallet]

LGTM. Would be nice to have a second reviewer though.

[rudolf]

I think the "TestUtils" section is the closest to addressing #51150:

1. Make it possible to use jest mocks for services so functional tests aren't required to test glue logic
2. Test that the route path is correctly registered including any path parameters
3. Test validation on request body and query parameters
4. Make assertions against response bodies and HTTP status code

Your "TestUtils" example shows how to do (2) - (4) (or in the case of path parameters, it's obvious how to do it yourself), but it doesn't cover (1). In your example it's easy to test the controller logic that catches the MisformedTextError, but if we're trying to test controller logic that catches a SavedObjectNotFoundError that gets thrown by a service/model that works over the network, it's not obvious how to achieve that. Some pseudo code that I think could solve this...

import {dashboardModel} from './dashboardModel';
router.get(
  {
    path: '/myPlugin/dashboard/{id}',
    validate: {
      params: schema.object({
        id: schema.string({ maxLength: 100 }),
      }),
    },
  },
  async (context, request, response) => {
    try {
      const dashboard = await dashboardModel.find(request.params.id);
      return response.ok({ body: JSON.stringify(dashboard) });
    } catch(error) {
      if (error instanceof SavedObjectNotFoundError) {
        return response.badRequest({ body: error.message })
      }
    }
  }
);

// src/plugins/my_plugin/server/integration_tests/formatter.test.ts
jest.mock('../dashboardModel');

import { dashboardModel } from '../dashboardModel';
import * as kbnTestServer from 'src/test_utils/kbn_server';
dashboardModel.find.mockRejectedValueOnce(new SavedObjectNotFoundError());

describe('myPlugin', () => {
  describe('GET /myPlugin/dashboard', () => {
    let root: ReturnType<typeof kbnTestServer.createRoot>;
    beforeAll(async () => {
      root = kbnTestServer.createRoot();
      await root.setup();
      await root.start();
    }, 30000);

    afterAll(async () => await root.shutdown());
    it('returns 404 if dashboard not found', async () => {
      const response = await kbnTestServer.request
        .get(root, '/myPlugin/dashboard/10')
        .expect(404);

      expect(response.body).toHaveProperty('message');
    });
  });
});

Something else that could be useful would be to mock Core services or other plugins to test your controller logic, again I haven't tried this...

// src/plugins/my_plugin/server/integration_tests/formatter.test.ts
import * as kbnTestServer from 'src/test_utils/kbn_server';

describe('myPlugin', () => {
  describe('GET /myPlugin/dashboard', () => {
    let root: ReturnType<typeof kbnTestServer.createRoot>;
    let start;
    beforeAll(async () => {
      root = kbnTestServer.createRoot();
      await root.setup();
      start = await root.start();
      start.plugins.data = dataPluginMock;
    }, 30000);

    afterAll(async () => await root.shutdown());
    it('returns 404 if data plugin cannot locate index pattern', async () => {
      start.plugins.data.getIndexPattern.mockRejectedValueOnce(new NotFoundError());
      const response = await kbnTestServer.request
        .get(root, '/myPlugin/myroute/10')
        .expect(404);

      expect(response.body).toHaveProperty('message');
    });
  });
});

[rudolf]

I think this document will be a really useful reference!

[rudolf]

Although it's a form of integration testing, I wonder if we shouldn't move this out of the "Integration tests" section into it's own section called "Controller testing" or "controller integration testing". ITO the testing pyramid we would have from bottom to top: unit testing, controller testing, integration testing. Because it's a very lightweight type of integration test we should encourage more "controller testing" than fullstack integration tests.

[mshustov]

TestUtils is not limited by controller testing. For example, you can use it to test any data kept in memory by your plugin (registry pattern).

[mshustov]

but if we're trying to test controller logic that catches a SavedObjectNotFoundError that gets thrown by a service/model that works over the network, it's not obvious how to achieve that.

Wouldn't it be enough to test it similarly to MisformedTextError?

// plugin
try{
  await model.do(savedObjectsClient)
} catch(error) {
  if (SavedObjectsErrorHelpers.isNotFoundError(error)) {
    return response.badRequest({ body: error.message })
  }
}
// test
it('allows SavedObjects "Not found" error to bubble up', async () => {
  jest.assertion(1);
  try{
    await model.do()
  } catch(error){
    expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true);
  }
});

Regarding the two options that you proposed:

• I think it's fine to mock your plugin internals for the integration test as you did in the first example. I will add a similar example to the guide.
• There are lots of internal dependencies, and plugin mocking the platform services makes integration tests fragile. I'd avoid suggesting the second proposal.

WDYT?

[rudolf]

❤️

[rudolf]

Wouldn't it be enough to test it similarly to MisformedTextError?

I like the example you added 👍

There are lots of internal dependencies, and plugin mocking the platform services makes integration tests fragile. I'd avoid suggesting the second proposal.
WDYT?

I agree, it's better to create an abstraction between your controller and dependencies rather than directly calling Core/Plugin API's from the controller.