Open sorenlouv opened 1 year ago
Pinging @elastic/apm-ui (Team:APM)
We can simply remove the Unit Test for
The API tests for these were added as part of this PR
@achyutjhunjhunwala Sounds good! Will you remove and update this issue? Thanks!
After further analysing, i can confirm that we can remove the Unit Tests for
PR can be found here.
Regarding the Units Test for server/routes/environments/get_all_environments.test.ts
, this function is used by Agent Configuration ('GET /api/apm/settings/agent-configuration/environments')
and Anomaly Detection ('GET /internal/apm/settings/anomaly-detection/environments')
routes and i see both of them are missing API tests. Hence i will keep this route in this list
In the past we didn't have API test so on the server we'd mock out Elasticsearch using
mockApmEventClient
. This way we could at test the data transformation happening after the ES call. The downside of this is of course that any changes to the actual ES query will go completely unnoticed. Today we have a solid API test suite so we should convert unit tests that mocks out the ES request. This is easy to find. Simply search formockApmEventClient
. The only exception I found waslib/helpers/transactions/get_is_using_transaction_events.test.ts
which might be a valid use case to keep as a unit testThe following unit tests could be converted to API tests:
server/routes/environments/get_environments.test.ts