Add integration test for the /override API #195
Conversation
| Request request = new Request("GET", | ||
| "/_opendistro/_performanceanalyzer/metrics/?metrics=Disk_Utilization&agg=max&dim=&nodes=all"); | ||
| Response resp = paClient.performRequest(request); | ||
| Assert.assertEquals(HttpStatus.SC_OK, resp.getStatusLine().getStatusCode()); | ||
| jsonString[0] = EntityUtils.toString(resp.getEntity()); | ||
| JsonNode root = mapper.readTree(jsonString[0]); |
There was a problem hiding this comment.
This will be very frequently used. Therefore, do you think we should create a helper method in PerformanceAnalyzerIntegTestBase that takes URI and request PARAMS and then converts the response to Json and returns ?
There was a problem hiding this comment.
I agree that this would be very useful and it's much better than ad hoc JSON parsing, but I think that can be a near future iteration and doesn't have to be a part of this PR
...om/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerIntegTestBase.java
Outdated
Show resolved
Hide resolved
| Request request = new Request("GET", | ||
| "/_opendistro/_performanceanalyzer/metrics/?metrics=Disk_Utilization&agg=max&dim=&nodes=all"); | ||
| Response resp = paClient.performRequest(request); | ||
| Assert.assertEquals(HttpStatus.SC_OK, resp.getStatusLine().getStatusCode()); | ||
| jsonString[0] = EntityUtils.toString(resp.getEntity()); | ||
| JsonNode root = mapper.readTree(jsonString[0]); |
There was a problem hiding this comment.
I agree that this would be very useful and it's much better than ad hoc JSON parsing, but I think that can be a near future iteration and doesn't have to be a part of this PR
| @Test | ||
| public void testRcaIsRunning() throws Exception { | ||
| ensurePaAndRcaEnabled(); | ||
| WaitFor.waitFor(() -> { | ||
| Request request = new Request("GET", "/_opendistro/_performanceanalyzer/rca"); | ||
| try { | ||
| Response resp = paClient.performRequest(request); | ||
| return Objects.equals(HttpStatus.SC_OK, resp.getStatusLine().getStatusCode()); | ||
| } catch (Exception e) { // 404, RCA context hasn't been set up yet | ||
| return false; | ||
| } | ||
| }, 2, TimeUnit.MINUTES); | ||
| } |
There was a problem hiding this comment.
Could we make this a utility method in the base class? It doesn't have to be a test
There was a problem hiding this comment.
I feel it's a good test!
src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/ConfigOverridesIT.java
Outdated
Show resolved
Hide resolved
Fixes #, if available:
#196
Description of changes:
Adds integration test for the override api
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.