From 2cb472bac00eabe032637dc3793d1b55455abc12 Mon Sep 17 00:00:00 2001 From: Jacob Choi Date: Thu, 19 Oct 2023 21:05:22 -0400 Subject: [PATCH 1/3] testing changes Signed-off-by: Jacob Choi added signoff --- .../alerting/MonitorRunnerServiceIT.kt | 26 ++++++++++++++----- .../alerting/alerts/AlertIndicesIT.kt | 8 ++++-- .../bwc/AlertingBackwardsCompatibilityIT.kt | 6 ++++- .../alerting/resthandler/MonitorRestApiIT.kt | 20 +++++++++++--- .../resthandler/SecureDestinationRestApiIT.kt | 4 +-- .../SecureEmailAccountRestApiIT.kt | 4 +-- .../resthandler/SecureEmailGroupsRestApiIT.kt | 4 +-- .../resthandler/SecureMonitorRestApiIT.kt | 4 +-- .../resthandler/SecureWorkflowRestApiIT.kt | 4 +-- .../DestinationMigrationUtilServiceIT.kt | 6 ++++- 10 files changed, 62 insertions(+), 24 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt index a56129850..aaf9cdce3 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt @@ -45,6 +45,7 @@ import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder import org.opensearch.search.aggregations.metrics.CardinalityAggregationBuilder import org.opensearch.search.aggregations.support.MultiTermsValuesSourceConfig import org.opensearch.search.builder.SearchSourceBuilder +import org.opensearch.test.OpenSearchTestCase import java.net.URLEncoder import java.time.Instant import java.time.ZonedDateTime @@ -53,6 +54,7 @@ import java.time.temporal.ChronoUnit import java.time.temporal.ChronoUnit.DAYS import java.time.temporal.ChronoUnit.MILLIS import java.time.temporal.ChronoUnit.MINUTES +import java.util.concurrent.TimeUnit import kotlin.collections.HashMap class MonitorRunnerServiceIT : AlertingRestTestCase() { @@ -137,7 +139,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { verifyAlert(firstRunAlert, monitor) // Runner uses ThreadPool.CachedTimeThread thread which only updates once every 200 ms. Wait a bit to // see lastNotificationTime change. - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 200, TimeUnit.MILLISECONDS) executeMonitor(monitor.id) val secondRunAlert = searchAlerts(monitor).single() verifyAlert(secondRunAlert, monitor) @@ -239,7 +243,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { // Runner uses ThreadPool.CachedTimeThread thread which only updates once every 200 ms. Wait a bit to // let lastNotificationTime change. W/o this sleep the test can result in a false negative. - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 200, TimeUnit.MILLISECONDS) val response = executeMonitor(monitor.id) val output = entityAsMap(response) @@ -739,7 +745,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { verifyAlert(activeAlert1.single(), monitor, ACTIVE) val actionResults1 = verifyActionExecutionResultInAlert(activeAlert1[0], mutableMapOf(Pair(actionThrottleEnabled.id, 0))) - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 200, TimeUnit.MILLISECONDS) updateMonitor(monitor.copy(triggers = listOf(trigger.copy(condition = NEVER_RUN)), id = monitor.id)) executeMonitor(monitor.id) val completedAlert = searchAlerts(monitor, AlertIndices.ALL_ALERT_INDEX_PATTERN).single() @@ -1372,7 +1380,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { // Runner uses ThreadPool.CachedTimeThread thread which only updates once every 200 ms. Wait a bit to // let lastNotificationTime change. W/o this sleep the test can result in a false negative. - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 200, TimeUnit.MILLISECONDS) executeMonitor(monitor.id) // Check that the lastNotification time of the acknowledged Alert wasn't updated and the active Alert's was @@ -1392,7 +1402,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { ) // Execute Monitor and check that both Alerts were updated - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil (searchAlerts(monitor, AlertIndices.ALL_ALERT_INDEX_PATTERN).size == 2) + }, 200, TimeUnit.MILLISECONDS) executeMonitor(monitor.id) currentAlerts = searchAlerts(monitor, AlertIndices.ALL_ALERT_INDEX_PATTERN) val completedAlerts = currentAlerts.filter { it.state == COMPLETED } @@ -1914,7 +1926,9 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { // Runner uses ThreadPool.CachedTimeThread thread which only updates once every 200 ms. Wait a bit to // let Action executionTime change. W/o this sleep the test can result in a false negative. - Thread.sleep(200) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 200, TimeUnit.MILLISECONDS) val monitorRunResultThrottled = entityAsMap(executeMonitor(monitor.id)) verifyActionThrottleResultsForBucketLevelMonitor( monitorRunResult = monitorRunResultThrottled, diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/alerts/AlertIndicesIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/alerts/AlertIndicesIT.kt index b058da877..3bc702616 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/alerts/AlertIndicesIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/alerts/AlertIndicesIT.kt @@ -140,7 +140,9 @@ class AlertIndicesIT : AlertingRestTestCase() { executeMonitor(trueMonitor) // Allow for a rollover index. - Thread.sleep(2000) + OpenSearchTestCase.waitUntil({ + return@waitUntil (getAlertIndices().size >= 3) + }, 2, TimeUnit.SECONDS) assertTrue("Did not find 3 alert indices", getAlertIndices().size >= 3) } @@ -157,7 +159,9 @@ class AlertIndicesIT : AlertingRestTestCase() { executeMonitor(trueMonitor.id) // Allow for a rollover index. - Thread.sleep(2000) + OpenSearchTestCase.waitUntil({ + return@waitUntil (getFindingIndices().size >= 2) + }, 2, TimeUnit.SECONDS) assertTrue("Did not find 2 alert indices", getFindingIndices().size >= 2) } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/bwc/AlertingBackwardsCompatibilityIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/bwc/AlertingBackwardsCompatibilityIT.kt index 937be869d..772ea3eea 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/bwc/AlertingBackwardsCompatibilityIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/bwc/AlertingBackwardsCompatibilityIT.kt @@ -16,6 +16,8 @@ import org.opensearch.commons.alerting.model.Monitor import org.opensearch.core.rest.RestStatus import org.opensearch.index.query.QueryBuilders import org.opensearch.search.builder.SearchSourceBuilder +import org.opensearch.test.OpenSearchTestCase +import java.util.concurrent.TimeUnit class AlertingBackwardsCompatibilityIT : AlertingRestTestCase() { @@ -69,7 +71,9 @@ class AlertingBackwardsCompatibilityIT : AlertingRestTestCase() { // the test execution by a lot (might have to wait for Job Scheduler plugin integration first) // Waiting a minute to ensure the Monitor ran again at least once before checking if the job is running // on time - Thread.sleep(60000) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 1, TimeUnit.MINUTES) verifyMonitorStats("/_plugins/_alerting") } } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt index 5450081c7..5176fc2b1 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -820,7 +820,10 @@ class MonitorRestApiIT : AlertingRestTestCase() { assertEquals("Delete request not successful", RestStatus.OK, deleteResponse.restStatus()) // Wait 5 seconds for event to be processed and alerts moved - Thread.sleep(5000) + OpenSearchTestCase.waitUntil({ + val historyAlerts = searchAlerts(monitor, AlertIndices.ALERT_HISTORY_WRITE_INDEX) + return@waitUntil (historyAlerts.size == 1) + }, 5, TimeUnit.SECONDS) val alerts = searchAlerts(monitor) assertEquals("Active alert was not deleted", 0, alerts.size) @@ -851,7 +854,10 @@ class MonitorRestApiIT : AlertingRestTestCase() { assertEquals("Update request not successful", RestStatus.OK, updateResponse.restStatus()) // Wait 5 seconds for event to be processed and alerts moved - Thread.sleep(5000) + OpenSearchTestCase.waitUntil({ + val historyAlerts = searchAlerts(monitor, AlertIndices.ALERT_HISTORY_WRITE_INDEX) + return@waitUntil (historyAlerts.size == 1) + }, 5, TimeUnit.SECONDS) val alerts = searchAlerts(monitor) assertEquals("Active alert was not deleted", 0, alerts.size) @@ -881,7 +887,11 @@ class MonitorRestApiIT : AlertingRestTestCase() { assertEquals("Update request not successful", RestStatus.OK, updateResponse.restStatus()) // Wait 5 seconds for event to be processed and alerts moved - Thread.sleep(5000) + OpenSearchTestCase.waitUntil({ + val alerts = searchAlerts(monitor) + val historyAlerts = searchAlerts(monitor, AlertIndices.ALERT_HISTORY_WRITE_INDEX) + return@waitUntil (alerts.isEmpty() && historyAlerts.size == 1) + }, 5, TimeUnit.SECONDS) val alerts = searchAlerts(monitor) assertEquals("Active alert was not deleted", 0, alerts.size) @@ -1002,7 +1012,9 @@ class MonitorRestApiIT : AlertingRestTestCase() { enableScheduledJob() // Sleep briefly so sweep can reschedule the Monitor - Thread.sleep(2000) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 2, TimeUnit.SECONDS) alertingStats = getAlertingStats() assertAlertingStatsSweeperEnabled(alertingStats, true) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt index d3650460b..69277c388 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt @@ -7,10 +7,10 @@ package org.opensearch.alerting.resthandler import org.apache.http.HttpHeaders import org.apache.http.message.BasicHeader +import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix import org.junit.After import org.junit.Before import org.junit.BeforeClass -import org.junit.Ignore import org.opensearch.alerting.ALERTING_GET_DESTINATION_ACCESS import org.opensearch.alerting.AlertingPlugin import org.opensearch.alerting.AlertingRestTestCase @@ -30,7 +30,7 @@ import org.opensearch.test.junit.annotations.TestLogging import java.time.Instant // TODO investigate flaky nature of tests. not reproducible in local but fails in jenkins CI -@Ignore +@AwaitsFix(bugUrl = "some url") @TestLogging("level:DEBUG", reason = "Debug for tests.") @Suppress("UNCHECKED_CAST") class SecureDestinationRestApiIT : AlertingRestTestCase() { diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt index b8f13f296..9436c16ee 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt @@ -9,10 +9,10 @@ import org.apache.http.HttpHeaders import org.apache.http.entity.ContentType import org.apache.http.entity.StringEntity import org.apache.http.message.BasicHeader +import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix import org.junit.After import org.junit.Before import org.junit.BeforeClass -import org.junit.Ignore import org.opensearch.alerting.ALERTING_GET_EMAIL_ACCOUNT_ACCESS import org.opensearch.alerting.ALERTING_NO_ACCESS_ROLE import org.opensearch.alerting.ALERTING_SEARCH_EMAIL_ACCOUNT_ACCESS @@ -43,7 +43,7 @@ val SEARCH_EMAIL_ACCOUNT_DSL = """ """.trimIndent() // TODO investigate flaky nature of tests. not reproducible in local but fails in jenkins CI -@Ignore +@AwaitsFix(bugUrl = "some url") class SecureEmailAccountRestApiIT : AlertingRestTestCase() { companion object { diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt index 0c1fc85f6..60ac455c6 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt @@ -9,10 +9,10 @@ import org.apache.http.HttpHeaders import org.apache.http.entity.ContentType import org.apache.http.entity.StringEntity import org.apache.http.message.BasicHeader +import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix import org.junit.After import org.junit.Before import org.junit.BeforeClass -import org.junit.Ignore import org.opensearch.alerting.ALERTING_GET_EMAIL_GROUP_ACCESS import org.opensearch.alerting.ALERTING_SEARCH_EMAIL_GROUP_ACCESS import org.opensearch.alerting.AlertingPlugin @@ -42,7 +42,7 @@ val SEARCH_EMAIL_GROUP_DSL = """ """.trimIndent() // TODO investigate flaky nature of tests. not reproducible in local but fails in jenkins CI -@Ignore +@AwaitsFix(bugUrl = "some url") @TestLogging("level:DEBUG", reason = "Debug for tests.") @Suppress("UNCHECKED_CAST") class SecureEmailGroupsRestApiIT : AlertingRestTestCase() { diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt index a52062d9d..aee4c5a2a 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -10,10 +10,10 @@ import org.apache.http.entity.ContentType import org.apache.http.entity.StringEntity import org.apache.http.message.BasicHeader import org.apache.http.nio.entity.NStringEntity +import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix import org.junit.After import org.junit.Before import org.junit.BeforeClass -import org.junit.Ignore import org.opensearch.alerting.ADMIN import org.opensearch.alerting.ALERTING_BASE_URI import org.opensearch.alerting.ALERTING_DELETE_MONITOR_ACCESS @@ -68,7 +68,7 @@ import org.opensearch.search.builder.SearchSourceBuilder import org.opensearch.test.junit.annotations.TestLogging // TODO investigate flaky nature of tests. not reproducible in local but fails in jenkins CI -@Ignore +@AwaitsFix(bugUrl = "some url") @TestLogging("level:DEBUG", reason = "Debug for tests.") @Suppress("UNCHECKED_CAST") class SecureMonitorRestApiIT : AlertingRestTestCase() { diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureWorkflowRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureWorkflowRestApiIT.kt index 8984bdea7..53e532c15 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureWorkflowRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureWorkflowRestApiIT.kt @@ -9,10 +9,10 @@ import org.apache.http.HttpHeaders import org.apache.http.entity.ContentType import org.apache.http.message.BasicHeader import org.apache.http.nio.entity.NStringEntity +import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix import org.junit.After import org.junit.Before import org.junit.BeforeClass -import org.junit.Ignore import org.opensearch.alerting.ALERTING_BASE_URI import org.opensearch.alerting.ALERTING_DELETE_WORKFLOW_ACCESS import org.opensearch.alerting.ALERTING_EXECUTE_WORKFLOW_ACCESS @@ -63,7 +63,7 @@ import org.opensearch.test.junit.annotations.TestLogging import java.time.Instant // TODO investigate flaky nature of tests. not reproducible in local but fails in jenkins CI -@Ignore +@AwaitsFix(bugUrl = "some url") @TestLogging("level:DEBUG", reason = "Debug for tests.") @Suppress("UNCHECKED_CAST") class SecureWorkflowRestApiIT : AlertingRestTestCase() { diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/util/destinationmigration/DestinationMigrationUtilServiceIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/util/destinationmigration/DestinationMigrationUtilServiceIT.kt index f9c40e465..903eedb44 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/util/destinationmigration/DestinationMigrationUtilServiceIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/util/destinationmigration/DestinationMigrationUtilServiceIT.kt @@ -18,8 +18,10 @@ import org.opensearch.alerting.util.DestinationType import org.opensearch.client.ResponseException import org.opensearch.commons.alerting.model.ScheduledJob.Companion.SCHEDULED_JOBS_INDEX import org.opensearch.core.rest.RestStatus +import org.opensearch.test.OpenSearchTestCase import java.time.Instant import java.util.UUID +import java.util.concurrent.TimeUnit class DestinationMigrationUtilServiceIT : AlertingRestTestCase() { @@ -80,7 +82,9 @@ class DestinationMigrationUtilServiceIT : AlertingRestTestCase() { // Create cluster change event and wait for migration service to complete migrating data over client().updateSettings("indices.recovery.max_bytes_per_sec", "40mb") - Thread.sleep(120000) + OpenSearchTestCase.waitUntil({ + return@waitUntil false + }, 2, TimeUnit.MINUTES) for (id in ids) { val response = client().makeRequest( From f04c92824c7d76eac1619cd8b3faf6899337ad94 Mon Sep 17 00:00:00 2001 From: Jacob Choi Date: Fri, 20 Oct 2023 16:27:08 -0400 Subject: [PATCH 2/3] fixing error Signed-off-by: Jacob Choi --- .../kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt index aaf9cdce3..8da9c1e4c 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt @@ -1403,7 +1403,7 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() { // Execute Monitor and check that both Alerts were updated OpenSearchTestCase.waitUntil({ - return@waitUntil (searchAlerts(monitor, AlertIndices.ALL_ALERT_INDEX_PATTERN).size == 2) + return@waitUntil false }, 200, TimeUnit.MILLISECONDS) executeMonitor(monitor.id) currentAlerts = searchAlerts(monitor, AlertIndices.ALL_ALERT_INDEX_PATTERN) From eaae8e631b0d6dbc5931e7c2dedf49de5dbd9181 Mon Sep 17 00:00:00 2001 From: Jacob Choi Date: Fri, 20 Oct 2023 17:05:16 -0400 Subject: [PATCH 3/3] fixing param mismatch Signed-off-by: Jacob Choi --- .../org/opensearch/alerting/resthandler/MonitorRestApiIT.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt index 5176fc2b1..d019be5ec 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -821,8 +821,9 @@ class MonitorRestApiIT : AlertingRestTestCase() { // Wait 5 seconds for event to be processed and alerts moved OpenSearchTestCase.waitUntil({ + val alerts = searchAlerts(monitor) val historyAlerts = searchAlerts(monitor, AlertIndices.ALERT_HISTORY_WRITE_INDEX) - return@waitUntil (historyAlerts.size == 1) + return@waitUntil (alerts.isEmpty() && historyAlerts.size == 1) }, 5, TimeUnit.SECONDS) val alerts = searchAlerts(monitor) @@ -855,8 +856,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { // Wait 5 seconds for event to be processed and alerts moved OpenSearchTestCase.waitUntil({ - val historyAlerts = searchAlerts(monitor, AlertIndices.ALERT_HISTORY_WRITE_INDEX) - return@waitUntil (historyAlerts.size == 1) + return@waitUntil false }, 5, TimeUnit.SECONDS) val alerts = searchAlerts(monitor)