From 7458969f70d8477b43fe1082e8f1e3c27c9cd417 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 5 Aug 2024 21:32:37 -0400 Subject: [PATCH] Fix tests Signed-off-by: Craig Perkins --- .../reindex/remote/RemoteScrollableHitSourceTests.java | 2 ++ .../org/opensearch/common/logging/JsonLoggerTests.java | 9 ++++++--- .../org/opensearch/index/shard/RefreshListeners.java | 2 +- .../org/opensearch/plugins/TelemetryAwarePlugin.java | 4 ++-- server/src/test/java/org/opensearch/node/NodeTests.java | 4 ++-- .../admin/indices/RestValidateQueryActionTests.java | 7 +++++++ .../ThreadContextBasedTracerContextStorageTests.java | 5 ----- .../java/org/opensearch/test/OpenSearchTestCase.java | 2 +- 8 files changed, 21 insertions(+), 14 deletions(-) diff --git a/modules/reindex/src/test/java/org/opensearch/index/reindex/remote/RemoteScrollableHitSourceTests.java b/modules/reindex/src/test/java/org/opensearch/index/reindex/remote/RemoteScrollableHitSourceTests.java index 325de0cef1f8c..8aa66fc3cfd8c 100644 --- a/modules/reindex/src/test/java/org/opensearch/index/reindex/remote/RemoteScrollableHitSourceTests.java +++ b/modules/reindex/src/test/java/org/opensearch/index/reindex/remote/RemoteScrollableHitSourceTests.java @@ -606,6 +606,8 @@ protected Future doExecute( FutureCallback callback ) { try { + // Throw away the current thread context to simulate running async httpclient's thread pool + threadPool.getThreadContext().stashContext(); ClassicHttpRequest request = getRequest(requestProducer); URL resource = resources[responseCount]; String path = paths[responseCount++]; diff --git a/qa/logging-config/src/test/java/org/opensearch/common/logging/JsonLoggerTests.java b/qa/logging-config/src/test/java/org/opensearch/common/logging/JsonLoggerTests.java index 3a1d699aee863..7fbfd6929ebdf 100644 --- a/qa/logging-config/src/test/java/org/opensearch/common/logging/JsonLoggerTests.java +++ b/qa/logging-config/src/test/java/org/opensearch/common/logging/JsonLoggerTests.java @@ -408,8 +408,11 @@ protected List featureValueOf(JsonLogLine actual) { private void withThreadContext(CheckedConsumer consumer) throws Exception { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - HeaderWarning.setThreadContext(threadContext); - consumer.accept(threadContext); - HeaderWarning.removeThreadContext(threadContext); + try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + HeaderWarning.setThreadContext(threadContext); + consumer.accept(threadContext); + } finally { + HeaderWarning.removeThreadContext(threadContext); + } } } diff --git a/server/src/main/java/org/opensearch/index/shard/RefreshListeners.java b/server/src/main/java/org/opensearch/index/shard/RefreshListeners.java index aa3de72e4232e..803db773efe6c 100644 --- a/server/src/main/java/org/opensearch/index/shard/RefreshListeners.java +++ b/server/src/main/java/org/opensearch/index/shard/RefreshListeners.java @@ -99,7 +99,7 @@ public RefreshListeners( final IntSupplier getMaxRefreshListeners, final Runnable forceRefresh, final Logger logger, - ThreadContext threadContext, + final ThreadContext threadContext, final MeanMetric refreshMetric ) { this.getMaxRefreshListeners = getMaxRefreshListeners; diff --git a/server/src/main/java/org/opensearch/plugins/TelemetryAwarePlugin.java b/server/src/main/java/org/opensearch/plugins/TelemetryAwarePlugin.java index 01287931719b2..4069b2f9768f1 100644 --- a/server/src/main/java/org/opensearch/plugins/TelemetryAwarePlugin.java +++ b/server/src/main/java/org/opensearch/plugins/TelemetryAwarePlugin.java @@ -8,7 +8,7 @@ package org.opensearch.plugins; -import org.opensearch.client.node.NodeClient; +import org.opensearch.client.Client; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.annotation.ExperimentalApi; @@ -63,7 +63,7 @@ public interface TelemetryAwarePlugin { * @param contextSwitcher A client for switching to plugin system context */ default Collection createComponents( - NodeClient client, + Client client, ClusterService clusterService, ThreadPool threadPool, ResourceWatcherService resourceWatcherService, diff --git a/server/src/test/java/org/opensearch/node/NodeTests.java b/server/src/test/java/org/opensearch/node/NodeTests.java index 01c1a5c3b02d3..1a43185c7cde1 100644 --- a/server/src/test/java/org/opensearch/node/NodeTests.java +++ b/server/src/test/java/org/opensearch/node/NodeTests.java @@ -34,7 +34,7 @@ import org.apache.lucene.tests.util.LuceneTestCase; import org.opensearch.bootstrap.BootstrapCheck; import org.opensearch.bootstrap.BootstrapContext; -import org.opensearch.client.node.NodeClient; +import org.opensearch.client.Client; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.node.DiscoveryNodeRole; @@ -467,7 +467,7 @@ public MetricsRegistry getMetricsRegistry() { public static class MockTelemetryAwarePlugin extends Plugin implements TelemetryAwarePlugin { @Override public Collection createComponents( - NodeClient client, + Client client, ClusterService clusterService, ThreadPool threadPool, ResourceWatcherService resourceWatcherService, diff --git a/server/src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/server/src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java index 1468f5d0f0b61..3fb6764846da6 100644 --- a/server/src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java @@ -56,6 +56,7 @@ import org.opensearch.threadpool.ThreadPool; import org.opensearch.usage.UsageService; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import java.util.Collections; @@ -112,6 +113,12 @@ protected void doExecute(Task task, ActionRequest request, ActionListener listen controller.registerHandler(action); } + @Before + public void ensureCleanContext() { + // Make sure we have a clean context for each test + threadPool.getThreadContext().stashContext(); + } + @AfterClass public static void terminateThreadPool() throws InterruptedException { terminate(threadPool); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java index b48f02217f0d0..98dfc367c20f5 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java @@ -8,8 +8,6 @@ package org.opensearch.telemetry.tracing; -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; - import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; @@ -21,7 +19,6 @@ import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; -import org.opensearch.threadpool.ThreadPool; import org.junit.After; import org.junit.Before; @@ -41,10 +38,8 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; -@ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class ThreadContextBasedTracerContextStorageTests extends OpenSearchTestCase { private Tracer tracer; - private ThreadPool threadPool; private ThreadContext threadContext; private TracerContextStorage threadContextStorage; private ExecutorService executorService; diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java index e0d4f91355f88..6afc7c23d9e66 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java @@ -618,7 +618,7 @@ private void assertWarnings(boolean stripXContentPosition, List actualWa */ private void resetDeprecationLogger() { // "clear" context by stashing current values and dropping the returned StoredContext - // threadContext.stashContext(); + threadContext.stashContext(); } private static final List statusData = new ArrayList<>();