Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SOLR-16676: Test improvements #1443

Closed
wants to merge 2 commits into from
Closed

SOLR-16676: Test improvements #1443

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6387cf7
SOLR-16676: Test improvements
risdenk Mar 8, 2023
117960e
Avoid assertNotNull
risdenk Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 28 additions & 38 deletions solr/core/src/test/org/apache/solr/handler/TestHttpRequestId.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.slf4j.MDC;

@LogLevel("org.apache.solr.client.solrj.impl.Http2SolrClient=DEBUG")
@SuppressForbidden(reason = "We need to use log4J2 classes directly to test MDC impacts")
public class TestHttpRequestId extends SolrJettyTestBase {

@BeforeClass
Expand All @@ -45,79 +44,70 @@ public static void beforeTest() throws Exception {
}

@Test
public void mdcContextTest() throws Exception {
public void mdcContextTest() {
String collection = "/collection1";
BlockingQueue<Runnable> workQueue = new SynchronousQueue<Runnable>(false);
BlockingQueue<Runnable> workQueue = new SynchronousQueue<>(false);
setupClientAndRun(collection, workQueue);
}

@Test
public void mdcContextFailureTest() throws Exception {
public void mdcContextFailureTest() {
String collection = "/doesnotexist";
BlockingQueue<Runnable> workQueue = new SynchronousQueue<Runnable>(false);
BlockingQueue<Runnable> workQueue = new SynchronousQueue<>(false);
setupClientAndRun(collection, workQueue);
}

@Test
public void mdcContextTest2() throws Exception {
public void mdcContextTest2() {
String collection = "/collection1";
BlockingQueue<Runnable> workQueue = new ArrayBlockingQueue<Runnable>(10, false);
BlockingQueue<Runnable> workQueue = new ArrayBlockingQueue<>(10, false);
setupClientAndRun(collection, workQueue);
}

@Test
public void mdcContextFailureTest2() throws Exception {
public void mdcContextFailureTest2() {
String collection = "/doesnotexist";
BlockingQueue<Runnable> workQueue = new ArrayBlockingQueue<Runnable>(10, false);
BlockingQueue<Runnable> workQueue = new ArrayBlockingQueue<>(10, false);
setupClientAndRun(collection, workQueue);
}

@SuppressForbidden(reason = "We need to use log4J2 classes directly to test MDC impacts")
private void setupClientAndRun(String collection, BlockingQueue<Runnable> workQueue) {
String key = "mdcContextTestKey" + System.nanoTime();
String value = "TestHttpRequestId" + System.nanoTime();
final String key = "mdcContextTestKey" + System.nanoTime();
final String value = "TestHttpRequestId" + System.nanoTime();

AsyncListener<NamedList<Object>> listener =
new AsyncListener<>() {

@Override
public void onSuccess(NamedList<Object> t) {
assertTrue(value, value.equals(MDC.get(key)));
assertEquals(value, MDC.get(key));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good catch, much better to have the values included in the assertion message.

}

@Override
public void onFailure(Throwable throwable) {
assertTrue(value, value.equals(MDC.get(key)));
assertEquals(value, MDC.get(key));
}
};

try (LogListener reqLog =
LogListener.debug(Http2SolrClient.class).substring("response processing")) {

ThreadPoolExecutor commExecutor = null;
Http2SolrClient client = null;
try {
// client setup needs to be same as HttpShardHandlerFactory
commExecutor =
new ExecutorUtil.MDCAwareThreadPoolExecutor(
3,
Integer.MAX_VALUE,
1,
TimeUnit.SECONDS,
workQueue,
new SolrNamedThreadFactory("httpShardExecutor"),
false);
client =
new Http2SolrClient.Builder(jetty.getBaseUrl().toString() + collection)
.withExecutor(commExecutor)
.build();

// client setup needs to be same as HttpShardHandlerFactory
ThreadPoolExecutor commExecutor =
new ExecutorUtil.MDCAwareThreadPoolExecutor(
3,
Integer.MAX_VALUE,
1,
TimeUnit.SECONDS,
workQueue,
new SolrNamedThreadFactory("httpShardExecutor"),
false);
try (Http2SolrClient client =
new Http2SolrClient.Builder(jetty.getBaseUrl().toString() + collection)
.withExecutor(commExecutor)
.build()) {
MDC.put(key, value);
client.asyncRequest(new SolrPing(), null, listener);

} finally {
if (client != null) {
client.close();
}
ExecutorUtil.shutdownAndAwaitTermination(commExecutor);
MDC.remove(key);
}
Expand All @@ -126,7 +116,7 @@ public void onFailure(Throwable throwable) {
assertEquals(3, reqLog.getQueue().size());
while (!reqLog.getQueue().isEmpty()) {
var reqEvent = reqLog.getQueue().poll();
assertTrue(reqEvent.getContextData().containsKey(key));
assertNotNull(reqEvent);
risdenk marked this conversation as resolved.
Show resolved Hide resolved
assertEquals(value, reqEvent.getContextData().getValue(key));
}
}
Expand Down