From 407e061316e67bed0d4ac8357d6a23e1832aaf75 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Tue, 29 Mar 2022 18:18:07 -0700 Subject: [PATCH] Changed forbiddenAPIsTest files and made relevant forbidden fixes (#450) Signed-off-by: Amit Galitzky --- build.gradle | 10 +++--- src/forbidden/ad-test-signatures.txt | 16 +++++++++ ...ndexAnomalyDetectorActionHandlerTests.java | 28 +++++++++++----- ...dateAnomalyDetectorActionHandlerTests.java | 29 ---------------- .../ad/AnomalyDetectorJobRunnerTests.java | 6 +++- .../ad/AnomalyDetectorRestTestCase.java | 3 +- .../ad/HistoricalAnalysisRestTestCase.java | 2 +- .../opensearch/ad/NodeStateManagerTests.java | 11 +++++-- .../opensearch/ad/cluster/DailyCronTests.java | 6 +++- .../opensearch/ad/cluster/HashRingTests.java | 7 ++-- .../ad/cluster/HourlyCronTests.java | 6 +++- .../ad/cluster/MasterEventListenerTests.java | 6 +++- .../ad/e2e/DetectionResultEvalutationIT.java | 33 +++++++++++-------- .../opensearch/ad/ml/CheckpointDaoTests.java | 6 +++- .../ad/ml/CheckpointDeleteTests.java | 6 +++- .../ad/ml/EntityColdStarterTests.java | 2 +- .../ad/mock/plugin/MockReindexPlugin.java | 2 +- .../ad/plugin/MockReindexPlugin.java | 2 +- .../opensearch/ad/rest/ADRestTestUtils.java | 6 +++- .../ad/rest/AnomalyDetectorRestApiIT.java | 15 ++++----- .../ad/rest/HistoricalAnalysisRestApiIT.java | 2 +- .../ADResultBulkTransportActionTests.java | 15 +++++++-- .../ad/transport/AnomalyResultTests.java | 6 +++- .../opensearch/ad/transport/DeleteTests.java | 6 +++- ...exAnomalyDetectorTransportActionTests.java | 11 +++++-- ...ewAnomalyDetectorTransportActionTests.java | 6 +++- .../handler/AbstractIndexHandlerTest.java | 6 +++- .../handler/AnomalyResultHandlerTests.java | 11 +++++-- 28 files changed, 171 insertions(+), 94 deletions(-) create mode 100644 src/forbidden/ad-test-signatures.txt diff --git a/build.gradle b/build.gradle index 0ae29aaaa..65bb2c320 100644 --- a/build.gradle +++ b/build.gradle @@ -131,12 +131,10 @@ tasks.named('forbiddenApisMain').configure { signaturesFiles += files('src/forbidden/ad-signatures.txt') } -tasks.named('forbiddenApisTest').configure { - // Disable check because AD code has too many violations. - // For example, we have to allow @Test to be used in test classes not inherited from LuceneTestCase. - // see https://github.com/elastic/elasticsearch/blob/master/buildSrc/src/main/resources/forbidden/es-test-signatures.txt - ignoreFailures = true -} +//Adds custom file that only has some of the checks for testApis checks since too many violations +//For example, we have to allow @Test to be used in test classes not inherited from LuceneTestCase. +//example: warning for every file: `Forbidden annotation use: org.junit.Test [defaultMessage Just name your test method testFooBar]` +forbiddenApisTest.setSignaturesFiles(files('src/forbidden/ad-test-signatures.txt')) // Allow test cases to be named Tests without having to be inherited from LuceneTestCase. // see https://github.com/elastic/elasticsearch/blob/323f312bbc829a63056a79ebe45adced5099f6e6/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java diff --git a/src/forbidden/ad-test-signatures.txt b/src/forbidden/ad-test-signatures.txt new file mode 100644 index 000000000..045bba080 --- /dev/null +++ b/src/forbidden/ad-test-signatures.txt @@ -0,0 +1,16 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +@defaultMessage use format with Locale +java.lang.String#format(java.lang.String,java.lang.Object[]) +java.util.concurrent.ThreadLocalRandom + +java.security.MessageDigest#clone() @ use org.opensearch.common.hash.MessageDigests + +@defaultMessage Don't use MethodHandles in slow or lenient ways, use invokeExact instead. +java.lang.invoke.MethodHandle#invoke(java.lang.Object[]) +java.lang.invoke.MethodHandle#invokeWithArguments(java.lang.Object[]) +java.lang.invoke.MethodHandle#invokeWithArguments(java.util.List) +java.lang.Math#random() @ Use one of the various randomization methods from LuceneTestCase or OpenSearchTestCase for reproducibility diff --git a/src/test/java/org/opensearch/action/admin/indices/mapping/get/IndexAnomalyDetectorActionHandlerTests.java b/src/test/java/org/opensearch/action/admin/indices/mapping/get/IndexAnomalyDetectorActionHandlerTests.java index d9e0eb35a..9feaeee45 100644 --- a/src/test/java/org/opensearch/action/admin/indices/mapping/get/IndexAnomalyDetectorActionHandlerTests.java +++ b/src/test/java/org/opensearch/action/admin/indices/mapping/get/IndexAnomalyDetectorActionHandlerTests.java @@ -264,7 +264,7 @@ public void doE listener.onResponse((Response) response); } } catch (IOException e) { - e.printStackTrace(); + logger.error("Create field mapping threw an exception", e); } } }; @@ -345,7 +345,7 @@ public void doE listener.onResponse((Response) response); } } catch (IOException e) { - e.printStackTrace(); + logger.error("Create field mapping threw an exception", e); } } }; @@ -437,7 +437,7 @@ public void doE listener.onResponse((Response) response); } } catch (IOException e) { - e.printStackTrace(); + logger.error("Create field mapping threw an exception", e); } } }; @@ -527,7 +527,7 @@ public void doE listener.onResponse((Response) response); } } catch (IOException e) { - e.printStackTrace(); + logger.error("Create field mapping threw an exception", e); } } }; @@ -599,7 +599,10 @@ public void testTenMultiEntityDetectorsUpdateSingleEntityAdToMulti() throws IOEx doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof SearchRequest); assertTrue(args[1] instanceof ActionListener); @@ -613,7 +616,10 @@ public void testTenMultiEntityDetectorsUpdateSingleEntityAdToMulti() throws IOEx doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof GetRequest); assertTrue(args[1] instanceof ActionListener); @@ -675,7 +681,10 @@ public void testTenMultiEntityDetectorsUpdateExistingMultiEntityAd() throws IOEx doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof SearchRequest); assertTrue(args[1] instanceof ActionListener); @@ -689,7 +698,10 @@ public void testTenMultiEntityDetectorsUpdateExistingMultiEntityAd() throws IOEx doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof GetRequest); assertTrue(args[1] instanceof ActionListener); diff --git a/src/test/java/org/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java b/src/test/java/org/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java index f9923541f..dfd03633d 100644 --- a/src/test/java/org/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java +++ b/src/test/java/org/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java @@ -137,38 +137,9 @@ public void testValidateMoreThanThousandSingleEntityDetectorLimit() throws IOExc // extend NodeClient since its execute method is final and mockito does not allow to mock final methods // we can also use spy to overstep the final methods - NodeClient client = IndexAnomalyDetectorActionHandlerTests .getCustomNodeClient(detectorResponse, userIndexResponse, singleEntityDetector, threadPool); - // NodeClient client = new NodeClient(Settings.EMPTY, threadPool) { - // @Override - // public void doExecute( - // ActionType action, - // Request request, - // ActionListener listener - // ) { - // try { - // if (action.equals(SearchAction.INSTANCE)) { - // assertTrue(request instanceof SearchRequest); - // SearchRequest searchRequest = (SearchRequest) request; - // if (searchRequest.indices()[0].equals(ANOMALY_DETECTORS_INDEX)) { - // listener.onResponse((Response) detectorResponse); - // } else { - // listener.onResponse((Response) userIndexResponse); - // } - // } else { - // GetFieldMappingsResponse response = new GetFieldMappingsResponse( - // TestHelpers.createFieldMappings(detector.getIndices().get(0), "timestamp", "date") - // ); - // listener.onResponse((Response) response); - // } - // } catch (IOException e) { - // e.printStackTrace(); - // } - // } - // }; - NodeClient clientSpy = spy(client); handler = new ValidateAnomalyDetectorActionHandler( diff --git a/src/test/java/org/opensearch/ad/AnomalyDetectorJobRunnerTests.java b/src/test/java/org/opensearch/ad/AnomalyDetectorJobRunnerTests.java index 80e7030eb..a56addd72 100644 --- a/src/test/java/org/opensearch/ad/AnomalyDetectorJobRunnerTests.java +++ b/src/test/java/org/opensearch/ad/AnomalyDetectorJobRunnerTests.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Iterator; +import java.util.Locale; import java.util.concurrent.ExecutorService; import java.util.concurrent.ThreadFactory; @@ -174,7 +175,10 @@ public void setup() throws Exception { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); IndexRequest request = null; ActionListener listener = null; diff --git a/src/test/java/org/opensearch/ad/AnomalyDetectorRestTestCase.java b/src/test/java/org/opensearch/ad/AnomalyDetectorRestTestCase.java index c1b07fb6f..b344cb3c2 100644 --- a/src/test/java/org/opensearch/ad/AnomalyDetectorRestTestCase.java +++ b/src/test/java/org/opensearch/ad/AnomalyDetectorRestTestCase.java @@ -18,6 +18,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collections; +import java.util.Locale; import java.util.Map; import org.apache.http.HttpHeaders; @@ -196,7 +197,7 @@ protected Response previewAnomalyDetector(String detectorId, RestClient client, .makeRequest( client, "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null diff --git a/src/test/java/org/opensearch/ad/HistoricalAnalysisRestTestCase.java b/src/test/java/org/opensearch/ad/HistoricalAnalysisRestTestCase.java index 25a07882f..ae10b29cd 100644 --- a/src/test/java/org/opensearch/ad/HistoricalAnalysisRestTestCase.java +++ b/src/test/java/org/opensearch/ad/HistoricalAnalysisRestTestCase.java @@ -253,7 +253,7 @@ protected List waitUntilTaskReachState(String detectorId, Set ta try { adTaskProfile = getADTaskProfile(detectorId); } catch (Exception e) { - e.printStackTrace(); + logger.error("failed to get ADTaskProfile", e); } finally { Thread.sleep(1000); } diff --git a/src/test/java/org/opensearch/ad/NodeStateManagerTests.java b/src/test/java/org/opensearch/ad/NodeStateManagerTests.java index 85a34aa76..646780ea2 100644 --- a/src/test/java/org/opensearch/ad/NodeStateManagerTests.java +++ b/src/test/java/org/opensearch/ad/NodeStateManagerTests.java @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.Locale; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -147,7 +148,10 @@ private String setupDetector() throws IOException { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); GetRequest request = null; ActionListener listener = null; @@ -175,7 +179,10 @@ private void setupCheckpoint(boolean responseExists) throws IOException { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); GetRequest request = null; ActionListener listener = null; diff --git a/src/test/java/org/opensearch/ad/cluster/DailyCronTests.java b/src/test/java/org/opensearch/ad/cluster/DailyCronTests.java index e33b69f3a..5bbb4cb7f 100644 --- a/src/test/java/org/opensearch/ad/cluster/DailyCronTests.java +++ b/src/test/java/org/opensearch/ad/cluster/DailyCronTests.java @@ -20,6 +20,7 @@ import java.time.Clock; import java.time.Duration; import java.util.Arrays; +import java.util.Locale; import org.opensearch.OpenSearchException; import org.opensearch.action.ActionListener; @@ -57,7 +58,10 @@ private void templateDailyCron(DailyCronTestExecutionMode mode) { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 3 + ); assertTrue(args[2] instanceof ActionListener); ActionListener listener = (ActionListener) args[2]; diff --git a/src/test/java/org/opensearch/ad/cluster/HashRingTests.java b/src/test/java/org/opensearch/ad/cluster/HashRingTests.java index 00a338432..dbfc0e26c 100644 --- a/src/test/java/org/opensearch/ad/cluster/HashRingTests.java +++ b/src/test/java/org/opensearch/ad/cluster/HashRingTests.java @@ -147,7 +147,7 @@ public void testGetOwningNode() throws UnknownHostException { // Circles for realtime AD will change as it's eligible to build for when its empty assertEquals("Wrong hash ring size for realtime AD", 2, hashRing.getNodesWithSameAdVersion(Version.V_1_1_0, true).size()); }, e -> { - e.printStackTrace(); + logger.error("building hash ring failed", e); assertFalse("Build hash ring failed", true); })); @@ -166,7 +166,8 @@ public void testGetOwningNode() throws UnknownHostException { // Circles for realtime AD will not change as it's eligible to rebuild assertEquals("Wrong hash ring size for realtime AD", 2, hashRing.getNodesWithSameAdVersion(Version.V_1_1_0, true).size()); }, e -> { - e.printStackTrace(); + logger.error("building hash ring failed", e); + assertFalse("Build hash ring failed", true); })); @@ -185,7 +186,7 @@ public void testGetOwningNode() throws UnknownHostException { ); assertEquals("Wrong hash ring size for realtime AD", 4, hashRing.getNodesWithSameAdVersion(Version.V_1_1_0, true).size()); }, e -> { - e.printStackTrace(); + logger.error("building hash ring failed", e); assertFalse("Failed to build hash ring", true); })); } diff --git a/src/test/java/org/opensearch/ad/cluster/HourlyCronTests.java b/src/test/java/org/opensearch/ad/cluster/HourlyCronTests.java index 19487f00e..7cb4b4865 100644 --- a/src/test/java/org/opensearch/ad/cluster/HourlyCronTests.java +++ b/src/test/java/org/opensearch/ad/cluster/HourlyCronTests.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.Locale; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -64,7 +65,10 @@ public void templateHourlyCron(HourlyCronTestExecutionMode mode) { Client client = mock(Client.class); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 3 + ); assertTrue(args[2] instanceof ActionListener); ActionListener listener = (ActionListener) args[2]; diff --git a/src/test/java/org/opensearch/ad/cluster/MasterEventListenerTests.java b/src/test/java/org/opensearch/ad/cluster/MasterEventListenerTests.java index e103f579a..86a88e62c 100644 --- a/src/test/java/org/opensearch/ad/cluster/MasterEventListenerTests.java +++ b/src/test/java/org/opensearch/ad/cluster/MasterEventListenerTests.java @@ -22,6 +22,7 @@ import java.time.Clock; import java.util.Arrays; import java.util.HashMap; +import java.util.Locale; import org.junit.Before; import org.opensearch.ad.AbstractADTest; @@ -83,7 +84,10 @@ public void testOnOffMaster() { public void testBeforeStop() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 1); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 1 + ); LifecycleListener listener = null; if (args[0] instanceof LifecycleListener) { diff --git a/src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java b/src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java index 16c136fe2..3a8493c60 100644 --- a/src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java +++ b/src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java @@ -19,6 +19,7 @@ import java.io.FileReader; import java.io.IOException; import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.text.SimpleDateFormat; import java.time.Clock; import java.time.Instant; @@ -84,8 +85,8 @@ private void verifyAnomaly( ) throws Exception { RestClient client = client(); - String dataFileName = String.format("data/%s.data", datasetName); - String labelFileName = String.format("data/%s.label", datasetName); + String dataFileName = String.format(Locale.ROOT, "data/%s.data", datasetName); + String labelFileName = String.format(Locale.ROOT, "data/%s.label", datasetName); List data = getData(dataFileName); List> anomalies = getAnomalyWindows(labelFileName); @@ -162,7 +163,7 @@ private double[] getTestResults( } } catch (Exception e) { errors++; - e.printStackTrace(); + logger.error("failed to get detection results", e); } } return new double[] { positives, truePositives, positiveAnomalies.size(), errors }; @@ -318,8 +319,8 @@ private String createDetector(String datasetName, int intervalMinutes, RestClien } private List> getAnomalyWindows(String labalFileName) throws Exception { - JsonArray windows = new JsonParser() - .parse(new FileReader(new File(getClass().getResource(labalFileName).toURI()))) + JsonArray windows = JsonParser + .parseReader(new FileReader(new File(getClass().getResource(labalFileName).toURI()), Charset.defaultCharset())) .getAsJsonArray(); List> anomalies = new ArrayList<>(windows.size()); for (int i = 0; i < windows.size(); i++) { @@ -417,7 +418,9 @@ private void waitAllSyncheticDataIngested(int expectedSize, String datasetName, // Expected response: // "_index":"synthetic","_type":"_doc","_id":"10080","_score":null,"_source":{"timestamp":"2019-11-08T00:00:00Z","Feature1":156.30028000000001,"Feature2":100.211205,"host":"host1"},"sort":[1573171200000]} Response response = client.performRequest(request); - JsonObject json = new JsonParser().parse(new InputStreamReader(response.getEntity().getContent())).getAsJsonObject(); + JsonObject json = JsonParser + .parseReader(new InputStreamReader(response.getEntity().getContent(), Charset.defaultCharset())) + .getAsJsonObject(); JsonArray hits = json.getAsJsonObject("hits").getAsJsonArray("hits"); if (hits != null && hits.size() == 1 @@ -437,8 +440,8 @@ private void setWarningHandler(Request request, boolean strictDeprecationMode) { } private List getData(String datasetFileName) throws Exception { - JsonArray jsonArray = new JsonParser() - .parse(new FileReader(new File(getClass().getResource(datasetFileName).toURI()))) + JsonArray jsonArray = JsonParser + .parseReader(new FileReader(new File(getClass().getResource(datasetFileName).toURI()), Charset.defaultCharset())) .getAsJsonArray(); List list = new ArrayList<>(jsonArray.size()); jsonArray.iterator().forEachRemaining(i -> list.add(i.getAsJsonObject())); @@ -447,7 +450,10 @@ private List getData(String datasetFileName) throws Exception { private Map getDetectionResult(String detectorId, Instant begin, Instant end, RestClient client) { try { - Request request = new Request("POST", String.format("/_opendistro/_anomaly_detection/detectors/%s/_run", detectorId)); + Request request = new Request( + "POST", + String.format(Locale.ROOT, "/_opendistro/_anomaly_detection/detectors/%s/_run", detectorId) + ); request .setJsonEntity( String.format(Locale.ROOT, "{ \"period_start\": %d, \"period_end\": %d }", begin.toEpochMilli(), end.toEpochMilli()) @@ -585,7 +591,7 @@ private void indexTrainData(String datasetName, List data, int train Thread.sleep(1_000); data.stream().limit(trainTestSplit).forEach(r -> { try { - Request req = new Request("POST", String.format("/%s/_doc/", datasetName)); + Request req = new Request("POST", String.format(Locale.ROOT, "/%s/_doc/", datasetName)); req.setJsonEntity(r.toString()); client.performRequest(req); } catch (Exception e) { @@ -618,7 +624,7 @@ public void testRestartHCADDetector() throws Exception { private void verifyRestart(String datasetName, int intervalMinutes, int shingleSize) throws Exception { RestClient client = client(); - String dataFileName = String.format("data/%s.data", datasetName); + String dataFileName = String.format(Locale.ROOT, "data/%s.data", datasetName); List data = getData(dataFileName); @@ -631,7 +637,7 @@ private void verifyRestart(String datasetName, int intervalMinutes, int shingleS // e.g., 2019-11-01T00:03:00Z String pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'"; - SimpleDateFormat simpleDateFormat = new SimpleDateFormat(pattern); + SimpleDateFormat simpleDateFormat = new SimpleDateFormat(pattern, Locale.ROOT); simpleDateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); // calculate the gap between current time and the beginning of last shingle // the gap is used to adjust input training data's time so that the last @@ -648,8 +654,7 @@ private void verifyRestart(String datasetName, int intervalMinutes, int shingleS // by the time we trigger the run API, a few seconds have passed. +5 to make the adjusted time more than current time. long gap = time.convert(diff, TimeUnit.MILLISECONDS) + 5; - Calendar c = Calendar.getInstance(); - c.setTimeZone(TimeZone.getTimeZone("UTC")); + Calendar c = Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT); // only change training data as we only need to make sure detector is fully initialized for (int i = 0; i < trainTestSplit; i++) { diff --git a/src/test/java/org/opensearch/ad/ml/CheckpointDaoTests.java b/src/test/java/org/opensearch/ad/ml/CheckpointDaoTests.java index f602d8d6c..5b3d4e935 100644 --- a/src/test/java/org/opensearch/ad/ml/CheckpointDaoTests.java +++ b/src/test/java/org/opensearch/ad/ml/CheckpointDaoTests.java @@ -33,6 +33,7 @@ import java.io.FileReader; import java.io.IOException; import java.net.URISyntaxException; +import java.nio.charset.Charset; import java.security.AccessController; import java.security.PrivilegedAction; import java.time.Clock; @@ -865,7 +866,10 @@ private Pair, Instant> setUp1_0Model(String checkpointFileNa URISyntaxException { String model = null; try ( - FileReader v1CheckpointFile = new FileReader(new File(getClass().getResource(checkpointFileName).toURI())); + FileReader v1CheckpointFile = new FileReader( + new File(getClass().getResource(checkpointFileName).toURI()), + Charset.defaultCharset() + ); BufferedReader rr = new BufferedReader(v1CheckpointFile) ) { model = rr.readLine(); diff --git a/src/test/java/org/opensearch/ad/ml/CheckpointDeleteTests.java b/src/test/java/org/opensearch/ad/ml/CheckpointDeleteTests.java index d4dfa3891..024c84837 100644 --- a/src/test/java/org/opensearch/ad/ml/CheckpointDeleteTests.java +++ b/src/test/java/org/opensearch/ad/ml/CheckpointDeleteTests.java @@ -19,6 +19,7 @@ import java.util.Arrays; import java.util.Collections; +import java.util.Locale; import org.apache.commons.pool2.impl.GenericObjectPool; import org.junit.After; @@ -129,7 +130,10 @@ public void delete_by_detector_id_template(DeleteExecutionMode mode) { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 3 + ); assertTrue(args[2] instanceof ActionListener); ActionListener listener = (ActionListener) args[2]; diff --git a/src/test/java/org/opensearch/ad/ml/EntityColdStarterTests.java b/src/test/java/org/opensearch/ad/ml/EntityColdStarterTests.java index 87ed9b95f..091af5299 100644 --- a/src/test/java/org/opensearch/ad/ml/EntityColdStarterTests.java +++ b/src/test/java/org/opensearch/ad/ml/EntityColdStarterTests.java @@ -739,7 +739,7 @@ private void accuracyTemplate(int detectorIntervalMins) throws Exception { .build(); long seed = new Random().nextLong(); - System.out.println("seed = " + seed); + LOG.info("seed = " + seed); // create labelled data MultiDimDataWithTime dataWithKeys = LabelledAnomalyGenerator .getMultiDimData(dataSize + detector.getShingleSize() - 1, 50, 100, 5, seed, baseDimension, false, trainTestSplit, delta); diff --git a/src/test/java/org/opensearch/ad/mock/plugin/MockReindexPlugin.java b/src/test/java/org/opensearch/ad/mock/plugin/MockReindexPlugin.java index 300f06f77..04d60e5a2 100644 --- a/src/test/java/org/opensearch/ad/mock/plugin/MockReindexPlugin.java +++ b/src/test/java/org/opensearch/ad/mock/plugin/MockReindexPlugin.java @@ -88,7 +88,7 @@ protected void doExecute(Task task, UpdateByQueryRequest request, ActionListener false ); } catch (IOException exception) { - exception.printStackTrace(); + logger.error(exception); } listener.onResponse(response); } diff --git a/src/test/java/org/opensearch/ad/plugin/MockReindexPlugin.java b/src/test/java/org/opensearch/ad/plugin/MockReindexPlugin.java index fc2b7e362..7e2597651 100644 --- a/src/test/java/org/opensearch/ad/plugin/MockReindexPlugin.java +++ b/src/test/java/org/opensearch/ad/plugin/MockReindexPlugin.java @@ -86,7 +86,7 @@ protected void doExecute(Task task, UpdateByQueryRequest request, ActionListener false ); } catch (IOException exception) { - exception.printStackTrace(); + logger.error(exception); } listener.onResponse(response); } diff --git a/src/test/java/org/opensearch/ad/rest/ADRestTestUtils.java b/src/test/java/org/opensearch/ad/rest/ADRestTestUtils.java index 69e83ed70..7002dddd4 100644 --- a/src/test/java/org/opensearch/ad/rest/ADRestTestUtils.java +++ b/src/test/java/org/opensearch/ad/rest/ADRestTestUtils.java @@ -33,6 +33,8 @@ import org.apache.http.HttpHeaders; import org.apache.http.message.BasicHeader; import org.apache.http.util.EntityUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.Logger; import org.opensearch.ad.TestHelpers; import org.opensearch.ad.mock.model.MockSimpleLog; import org.opensearch.ad.model.ADTask; @@ -50,6 +52,8 @@ //TODO: remove duplicate code in HistoricalAnalysisRestTestCase public class ADRestTestUtils { + protected static final Logger LOG = (Logger) LogManager.getLogger(ADRestTestUtils.class); + public enum DetectorType { SINGLE_ENTITY_DETECTOR, SINGLE_CATEGORY_HC_DETECTOR, @@ -472,7 +476,7 @@ public static ADTaskProfile waitUntilTaskReachState(RestClient client, String de try { adTaskProfile = getADTaskProfile(client, detectorId); } catch (Exception e) { - e.printStackTrace(); + LOG.error("failed to get ADTaskProfile", e); } finally { Thread.sleep(1000); } diff --git a/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java b/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java index fbeb4979e..cbbba92f5 100644 --- a/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java +++ b/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java @@ -501,7 +501,7 @@ public void testPreviewAnomalyDetector() throws Exception { .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null @@ -515,7 +515,7 @@ public void testPreviewAnomalyDetector() throws Exception { .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null @@ -538,7 +538,7 @@ public void testPreviewAnomalyDetectorWhichNotExist() throws Exception { .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null @@ -560,7 +560,7 @@ public void testExecuteAnomalyDetectorWithNullDetectorId() throws Exception { .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null @@ -580,7 +580,7 @@ public void testPreviewAnomalyDetectorWithDetector() throws Exception { .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null, @@ -605,7 +605,7 @@ public void testPreviewAnomalyDetectorWithDetectorAndNoFeatures() throws Excepti .makeRequest( client(), "POST", - String.format(TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), + String.format(Locale.ROOT, TestHelpers.AD_BASE_PREVIEW_URI, input.getDetectorId()), ImmutableMap.of(), TestHelpers.toHttpEntity(input), null @@ -1189,7 +1189,6 @@ public void testDeleteAnomalyDetectorWhileRunning() throws Exception { // Deleting detector should fail while its running Exception exception = expectThrows(IOException.class, () -> { deleteAnomalyDetector(detector.getDetectorId(), client()); }); - exception.printStackTrace(); Assert.assertTrue(exception.getMessage().contains("Detector is running")); } @@ -1492,7 +1491,7 @@ public void testValidateAnomalyDetectorWithWrongCategoryField() throws Exception .extractValue("detector", responseMap); assertEquals( "non-existing category", - String.format(AbstractAnomalyDetectorActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword"), + String.format(Locale.ROOT, AbstractAnomalyDetectorActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword"), messageMap.get("category_field").get("message") ); diff --git a/src/test/java/org/opensearch/ad/rest/HistoricalAnalysisRestApiIT.java b/src/test/java/org/opensearch/ad/rest/HistoricalAnalysisRestApiIT.java index d4f12c894..e3464729f 100644 --- a/src/test/java/org/opensearch/ad/rest/HistoricalAnalysisRestApiIT.java +++ b/src/test/java/org/opensearch/ad/rest/HistoricalAnalysisRestApiIT.java @@ -289,7 +289,7 @@ public void testSearchTasks() throws IOException, InterruptedException, IllegalA waitUntilTaskDone(detectorId); - String query = String.format("{\"query\":{\"term\":{\"detector_id\":{\"value\":\"%s\"}}}}", detectorId); + String query = String.format(Locale.ROOT, "{\"query\":{\"term\":{\"detector_id\":{\"value\":\"%s\"}}}}", detectorId); Response response = TestHelpers .makeRequest(client(), "POST", TestHelpers.AD_BASE_DETECTORS_URI + "/tasks/_search", ImmutableMap.of(), query, null); String searchResult = EntityUtils.toString(response.getEntity()); diff --git a/src/test/java/org/opensearch/ad/transport/ADResultBulkTransportActionTests.java b/src/test/java/org/opensearch/ad/transport/ADResultBulkTransportActionTests.java index 5608bc5df..3d7d60d0c 100644 --- a/src/test/java/org/opensearch/ad/transport/ADResultBulkTransportActionTests.java +++ b/src/test/java/org/opensearch/ad/transport/ADResultBulkTransportActionTests.java @@ -103,7 +103,10 @@ public void testSendAll() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 3 + ); assertTrue(args[1] instanceof BulkRequest); assertTrue(args[2] instanceof ActionListener); @@ -133,7 +136,10 @@ public void testSendPartial() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 3 + ); assertTrue(args[1] instanceof BulkRequest); assertTrue(args[2] instanceof ActionListener); @@ -166,7 +172,10 @@ public void testSendRandomPartial() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 3 + ); assertTrue(args[1] instanceof BulkRequest); assertTrue(args[2] instanceof ActionListener); diff --git a/src/test/java/org/opensearch/ad/transport/AnomalyResultTests.java b/src/test/java/org/opensearch/ad/transport/AnomalyResultTests.java index b85833274..d57095856 100644 --- a/src/test/java/org/opensearch/ad/transport/AnomalyResultTests.java +++ b/src/test/java/org/opensearch/ad/transport/AnomalyResultTests.java @@ -48,6 +48,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; @@ -262,7 +263,10 @@ public void setUp() throws Exception { when(client.threadPool().getThreadContext()).thenReturn(threadContext); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); IndexRequest request = null; ActionListener listener = null; diff --git a/src/test/java/org/opensearch/ad/transport/DeleteTests.java b/src/test/java/org/opensearch/ad/transport/DeleteTests.java index 5f7364607..12d1897c5 100644 --- a/src/test/java/org/opensearch/ad/transport/DeleteTests.java +++ b/src/test/java/org/opensearch/ad/transport/DeleteTests.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.function.Supplier; import org.hamcrest.Matchers; @@ -216,7 +217,10 @@ private enum DetectorExecutionMode { public void StopDetectorResponseTemplate(DetectorExecutionMode mode) throws Exception { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 3); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 3 + ); assertTrue(args[2] instanceof ActionListener); ActionListener listener = (ActionListener) args[2]; diff --git a/src/test/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportActionTests.java b/src/test/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportActionTests.java index 9447b3dbd..fd76d4bea 100644 --- a/src/test/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportActionTests.java +++ b/src/test/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportActionTests.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.Locale; import org.junit.Assert; import org.junit.Before; @@ -120,7 +121,10 @@ public void setUp() throws Exception { .createGetResponse(detector, detector.getDetectorId(), AnomalyDetector.ANOMALY_DETECTORS_INDEX); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof GetRequest); assertTrue(args[1] instanceof ActionListener); @@ -144,7 +148,10 @@ public void setUp() throws Exception { ); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof SearchRequest); assertTrue(args[1] instanceof ActionListener); diff --git a/src/test/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportActionTests.java b/src/test/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportActionTests.java index 6f19c1a6d..fe29bc72f 100644 --- a/src/test/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportActionTests.java +++ b/src/test/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportActionTests.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -317,7 +318,10 @@ public void testPreviewTransportActionNoContext() throws IOException, Interrupte .createGetResponse(detector, detector.getDetectorId(), AnomalyDetector.ANOMALY_DETECTORS_INDEX); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length == 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length == 2 + ); assertTrue(args[0] instanceof GetRequest); assertTrue(args[1] instanceof ActionListener); diff --git a/src/test/java/org/opensearch/ad/transport/handler/AbstractIndexHandlerTest.java b/src/test/java/org/opensearch/ad/transport/handler/AbstractIndexHandlerTest.java index af0303eab..ff9720b02 100644 --- a/src/test/java/org/opensearch/ad/transport/handler/AbstractIndexHandlerTest.java +++ b/src/test/java/org/opensearch/ad/transport/handler/AbstractIndexHandlerTest.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Locale; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -112,7 +113,10 @@ protected void setWriteBlockAdResultIndex(boolean blocked) { protected void setUpSavingAnomalyResultIndex(boolean anomalyResultIndexExists, IndexCreation creationResult) throws IOException { doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 1); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 1 + ); ActionListener listener = invocation.getArgument(0); assertTrue(listener != null); switch (creationResult) { diff --git a/src/test/java/org/opensearch/ad/transport/handler/AnomalyResultHandlerTests.java b/src/test/java/org/opensearch/ad/transport/handler/AnomalyResultHandlerTests.java index 693f9f915..dcee4030b 100644 --- a/src/test/java/org/opensearch/ad/transport/handler/AnomalyResultHandlerTests.java +++ b/src/test/java/org/opensearch/ad/transport/handler/AnomalyResultHandlerTests.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.time.Clock; import java.util.Arrays; +import java.util.Locale; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -70,7 +71,10 @@ public void testSavingAdResult() throws IOException { setUpSavingAnomalyResultIndex(false); doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); IndexRequest request = invocation.getArgument(0); ActionListener listener = invocation.getArgument(1); assertTrue(request != null && listener != null); @@ -186,7 +190,10 @@ private void savingFailureTemplate(boolean throwOpenSearchRejectedExecutionExcep doAnswer(invocation -> { Object[] args = invocation.getArguments(); - assertTrue(String.format("The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), args.length >= 2); + assertTrue( + String.format(Locale.ROOT, "The size of args is %d. Its content is %s", args.length, Arrays.toString(args)), + args.length >= 2 + ); IndexRequest request = invocation.getArgument(0); ActionListener listener = invocation.getArgument(1); assertTrue(request != null && listener != null);