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

Core should support testing route handlers/controllers #51150

Closed
rudolf opened this issue Nov 20, 2019 · 4 comments
Closed

Core should support testing route handlers/controllers #51150

rudolf opened this issue Nov 20, 2019 · 4 comments
Assignees
Labels
Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@rudolf
Copy link
Contributor

rudolf commented Nov 20, 2019

Summary

@jloleysens and I discussed testing controllers and although workarounds were possible in Legacy, Core doesn't expose a way to test controller logic in an isolated way.

Details

The upgrade assistant plugin follows a fairly common pattern of "thin" controllers which handles the HTTP domain and glues together services which implement the business logic. Unit tests on the services themselves give good code coverage on the business logic, but still doesn't give full assurance that everything works as expected.

The legacy plugin created it's own Hapi server and registered the API routes / controllers to this server. This allowed the plugin to test the glue logic and API return codes using mocks for the underlying services. An alternative testing strategy would be to write these tests as functional tests, but this is hard and slow since testing the glue logic would require things like an Elasticsearch cluster which has nodes running different versions.

/**
* Since these route callbacks are so thin, these serve simply as integration tests
* to ensure they're wired up to the lib functions correctly. Business logic is tested
* more thoroughly in the es_migration_apis test.
*/
describe('reindex API', () => {
const server = new Server();
server.plugins = {
elasticsearch: {
getCluster: () => ({ callWithRequest: jest.fn() } as any),
} as any,
xpack_main: {
info: {},
},
} as any;
server.config = () => ({ get: () => '' } as any);
server.decorate('request', 'getSavedObjectsClient', () => jest.fn());
const credentialStore = credentialStoreFactory();
const worker = {
includes: jest.fn(),
forceRefresh: jest.fn(),
} as any;
registerReindexIndicesRoutes(server, worker, credentialStore);

@jloleysens got far by creating a testing router, but this still doesn't test everything a route/controller is responsible for:

/**
* Creates a very crude mock of the new platform router implementation. This enables use to test
* controller/handler logic without making HTTP requests to an actual server. This does not enable
* us to test whether our paths actual match, only the response codes of controllers given certain
* inputs. This should be replaced by a more wholistic solution (like functional tests) eventually.
*
* This also bypasses any validation installed on the route.
*/

Core should enable plugins to test their controllers:

  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 codes
@rudolf rudolf added Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Nov 20, 2019
@elasticmachine
Copy link
Contributor

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

@mshustov
Copy link
Contributor

mshustov commented Dec 7, 2019

Can plugins use existing test_utils? It can bootstrap the new platform. We need to provide recommendations on mocking core services (elasticsearch, saved objects) or a network layer.
https://github.com/elastic/kibana/blob/master/src/legacy/server/http/integration_tests/default_route_provider_config.test.ts

@rudolf
Copy link
Contributor Author

rudolf commented Jan 8, 2020

Reporting also uses this pattern for testing their legacy controllers:

test(`returns 401 if not valid job type`, async () => {
mockServer.plugins.elasticsearch
.getCluster('admin')
.callWithInternalUser.mockReturnValue(Promise.resolve(getHits({ jobtype: 'invalidJobType' })));
registerJobInfoRoutes(mockServer, exportTypesRegistry, mockLogger);
const request = {
method: 'GET',
url: '/api/reporting/jobs/download/1',
};
const { statusCode } = await mockServer.inject(request);
expect(statusCode).toBe(401);
});

@joshdover joshdover assigned mshustov and unassigned rudolf Jan 14, 2020
@joshdover joshdover removed the discuss label Jan 14, 2020
@joshdover joshdover changed the title [Discuss] Core should support testing route handlers/controllers Core should support testing route handlers/controllers Jan 14, 2020
@mshustov
Copy link
Contributor

done in #54908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants