[Infra UI] Test for metrics GraphQL endpoint #24179
Conversation
package.json
Outdated
| @@ -83,6 +83,7 @@ | |||
| "angular-route": "1.4.7", | |||
| "angular-sanitize": "1.5.7", | |||
| "angular-sortable-view": "0.0.15", | |||
| "apollo-boost": "^0.1.16", | |||
There was a problem hiding this comment.
Should this be in devDependencies instead?
There was a problem hiding this comment.
Er... that's weird. Good catch.
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| import ApolloClient from 'apollo-boost'; // eslint-disable-line |
There was a problem hiding this comment.
We don't need to include apollo-boost at all if we import this...
| import ApolloClient from 'apollo-boost'; // eslint-disable-line | |
| import { InMemoryCache } from 'apollo-cache-inmemory'; | |
| import { ApolloClient } from 'apollo-client'; | |
| import { HttpLink } from 'apollo-link-http'; |
| } | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
...and configure the ApolloClient like this:
return new ApolloClient({
cache: new InMemoryCache(),
link: new HttpLink({
credentials: 'same-origin',
fetch,
headers: {
'kbn-xsrf': 'xxx',
},
uri: `${kbnURL}/api/infra/graphql`,
}),
});| */ | ||
|
|
||
| import expect from 'expect.js'; | ||
| import { metricsQuery } from '../../../../plugins/infra/public/containers/metrics/metrics.gql_query'; |
There was a problem hiding this comment.
As a practice in graphql tests, maybe we want to locally define the query so the whole test is more readable and less entangled with client code?
We should be able to just inline something here like
import gql from 'graphql-tag';
const metricsQuery = gql`
...
`;
There was a problem hiding this comment.
It does test the query which we're actually using from the UI, isn't this a good thing?
There was a problem hiding this comment.
In a way that's good, but I would almost consider that part of the e2e tests. To me it's very hard to answer the question "what aspect of the api does this test case cover?".
Maybe there's a way to have both: Have a test case like "should support the metric container query" per client-side query and other test cases that test specific things like "supports two metrics in one query" with a locally defined query.
Does that sound reasonable?
|
@weltenwort I converted just the tests to typescript and they seem to be working. The only thing I don't know how to do (or where) is to fix the rules for the test directory so we can use default exports as the test loader expects, so I just disabled the linter for that specific line. |
|
nice! the linter override for that reason seems to be common practice in the test files |
💚 Build Succeeded |
|
Apologies for the late comment: Can we use |
|
@skh I changed the names to |
💚 Build Succeeded |
* [Infra UI] Test for metrics GraphQL endpoint * Moving apollo-boost to devDeps * Converting tests to typescript * Renaming infraops to infra
This PR sets up the first API test for the metric GraphQL endpoint. This comes with:
I reused the same query we are using from the app so if something changes in there the test should blowup as expected by any good test 👍
To run the tests locally:
BTW... The extra added bonus of this test is that because the metrics endpoint just re-routes the requests through the TSVB backend we get testing of TSVB for free!