From 6fa1a258a63fe929f1993702e9b4da29b167c4dd Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 14 Dec 2023 13:06:42 -0500 Subject: [PATCH] Fix flaky test SlowIntegrationTests.testDelayInSecurityIndexInitialization (#3763) Signed-off-by: Craig Perkins (cherry picked from commit 7498eb00679c307b75db80f54476a01ace48fae9) --- DEVELOPER_GUIDE.md | 23 ++++++++- build.gradle | 3 ++ .../security/SlowIntegrationTests.java | 47 +++++++++++------- .../org/opensearch/security/util/Repeat.java | 25 ++++++++++ .../opensearch/security/util/RepeatRule.java | 48 +++++++++++++++++++ 5 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 src/test/java/org/opensearch/security/util/Repeat.java create mode 100644 src/test/java/org/opensearch/security/util/RepeatRule.java diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index d36670c830..a758f1934a 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -225,11 +225,30 @@ curl -XGET https://localhost:9200/_plugins/_security/authinfo -u 'admin:admin' - Launch IntelliJ IDEA, choose **Project from Existing Sources**, and select directory with Gradle build script (`build.gradle`). -## Running integration tests +## Running tests Locally these can be run with `./gradlew test` with detailed results being avaliable at `${project-root}/build/reports/tests/test/index.html`, or run through an IDEs JUnit test runner. -Integration tests are automatically run on all pull requests for all supported versions of the JDK. These must pass for change(s) to be merged. Detailed logs of these test results are avaliable by going to the GitHub action workflow's summary view and downloading the associated jdk version run of the tests, after extracting this file onto your local machine integration tests results are at `./tests/tests/index.html`. +Tests are automatically run on all pull requests for all supported versions of the JDK. These must pass for change(s) to be merged. Detailed logs of these test results are available by going to the GitHub Actions workflow summary view and downloading the workflow run of the tests. If you see multiple tests listed with different JDK versions, you can download the version with whichever JDK you are interested in. After extracting the test file on your local machine, integration tests results can be found at `./tests/tests/index.html`. + +### Running an individual test multiple times + +This repo has a `@Repeat` annotation which you can import to annotate a test to run many times repeatedly. To use the annotation, add the following code to your test suite. + +``` +@Rule +public RepeatRule repeatRule = new RepeatRule(); + +@Test +@Repeat(10) +public void testMethod() { + ... +} +``` + +## Running tests in the integrationTest package + +Tests in the integrationTest package can be run with `./gradlew integrationTest`. ### Bulk test runs To collect reliability data on test runs there is a manual GitHub action workflow called `Bulk Integration Test`. The workflow is started for a branch on this project or in a fork by going to [GitHub action workflows](https://github.com/opensearch-project/security/actions/workflows/integration-tests.yml) and selecting `Run Workflow`. diff --git a/build.gradle b/build.gradle index b1bd92f788..7bc3e434a0 100644 --- a/build.gradle +++ b/build.gradle @@ -670,6 +670,9 @@ dependencies { testImplementation "org.springframework:spring-beans:${spring_version}" testImplementation 'org.junit.jupiter:junit-jupiter:5.10.1' testImplementation 'org.junit.jupiter:junit-jupiter-api:5.10.1' + testImplementation('org.awaitility:awaitility:4.2.0') { + exclude(group: 'org.hamcrest', module: 'hamcrest') + } // Only osx-x86_64, osx-aarch_64, linux-x86_64, linux-aarch_64, windows-x86_64 are available if (osdetector.classifier in ["osx-x86_64", "osx-aarch_64", "linux-x86_64", "linux-aarch_64", "windows-x86_64"]) { testImplementation "io.netty:netty-tcnative-classes:2.0.61.Final" diff --git a/src/test/java/org/opensearch/security/SlowIntegrationTests.java b/src/test/java/org/opensearch/security/SlowIntegrationTests.java index 9fa97c3d36..dce36e0af3 100644 --- a/src/test/java/org/opensearch/security/SlowIntegrationTests.java +++ b/src/test/java/org/opensearch/security/SlowIntegrationTests.java @@ -29,13 +29,14 @@ import java.io.IOException; import com.google.common.collect.Lists; -import org.apache.http.HttpStatus; +import org.awaitility.Awaitility; import org.junit.Assert; import org.junit.Test; import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.node.info.NodesInfoRequest; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.cluster.health.ClusterHealthStatus; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; @@ -50,6 +51,9 @@ import org.opensearch.security.test.helper.rest.RestHelper; import org.opensearch.transport.Netty4Plugin; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThrows; + public class SlowIntegrationTests extends SingleClusterTest { @Test @@ -223,27 +227,34 @@ public void testDelayInSecurityIndexInitialization() throws Exception { .put(ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true) .put("cluster.routing.allocation.exclude._ip", "127.0.0.1") .build(); - try { + assertThrows(IOException.class, () -> { setup(Settings.EMPTY, null, settings, false); - Assert.fail("Expected IOException here due to red cluster state"); - } catch (IOException e) { - // Index request has a default timeout of 1 minute, adding buffer between nodes initialization and cluster health check - Thread.sleep(1000 * 80); - // Ideally, we would want to remove this cluster setting, but default settings cannot be removed. So overriding with a reserved - // IP address clusterHelper.nodeClient() .admin() - .cluster() - .updateSettings( - new ClusterUpdateSettingsRequest().transientSettings( - Settings.builder().put("cluster.routing.allocation.exclude._ip", "192.0.2.0").build() - ) - ); - this.clusterInfo = clusterHelper.waitForCluster(ClusterHealthStatus.GREEN, TimeValue.timeValueSeconds(10), 3); - } + .indices() + .create(new CreateIndexRequest("test-index").timeout(TimeValue.timeValueSeconds(10))) + .actionGet(); + clusterHelper.waitForCluster(ClusterHealthStatus.GREEN, TimeValue.timeValueSeconds(5), ClusterConfiguration.DEFAULT.getNodes()); + }); + // Ideally, we would want to remove this cluster setting, but default settings cannot be removed. So overriding with a reserved + // IP address + clusterHelper.nodeClient() + .admin() + .cluster() + .updateSettings( + new ClusterUpdateSettingsRequest().transientSettings( + Settings.builder().put("cluster.routing.allocation.exclude._ip", "192.0.2.0").build() + ) + ); + this.clusterInfo = clusterHelper.waitForCluster(ClusterHealthStatus.GREEN, TimeValue.timeValueSeconds(10), 3); RestHelper rh = nonSslRestHelper(); - Thread.sleep(10000); - Assert.assertEquals(HttpStatus.SC_OK, rh.executeGetRequest("", encodeBasicHeader("admin", "admin")).getStatusCode()); + Awaitility.await() + .alias("Wait until Security is initialized") + .until( + () -> rh.executeGetRequest("/_plugins/_security/health", encodeBasicHeader("admin", "admin")) + .getTextFromJsonBody("/status"), + equalTo("UP") + ); } } diff --git a/src/test/java/org/opensearch/security/util/Repeat.java b/src/test/java/org/opensearch/security/util/Repeat.java new file mode 100644 index 0000000000..be9c9fddf0 --- /dev/null +++ b/src/test/java/org/opensearch/security/util/Repeat.java @@ -0,0 +1,25 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.util; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.METHOD; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ METHOD, ANNOTATION_TYPE }) +public @interface Repeat { + int value() default 1; +} diff --git a/src/test/java/org/opensearch/security/util/RepeatRule.java b/src/test/java/org/opensearch/security/util/RepeatRule.java new file mode 100644 index 0000000000..387f0c17e5 --- /dev/null +++ b/src/test/java/org/opensearch/security/util/RepeatRule.java @@ -0,0 +1,48 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.util; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +public class RepeatRule implements TestRule { + + private static class RepeatStatement extends Statement { + private final Statement statement; + private final int repeat; + + public RepeatStatement(Statement statement, int repeat) { + this.statement = statement; + this.repeat = repeat; + } + + @Override + public void evaluate() throws Throwable { + for (int i = 0; i < repeat; i++) { + statement.evaluate(); + } + } + + } + + @Override + public Statement apply(Statement statement, Description description) { + Statement result = statement; + Repeat repeat = description.getAnnotation(Repeat.class); + if (repeat != null) { + int times = repeat.value(); + result = new RepeatStatement(statement, times); + } + return result; + } +}