madrob commented on a change in pull request #198: URL: https://github.com/apache/solr/pull/198#discussion_r665578729
########## File path: solr/core/src/test/org/apache/solr/handler/admin/ThreadDumpHandlerTest.java ########## @@ -95,12 +99,23 @@ public void doTestMonitor(final boolean checkBlockedThreadViaPolling) throws Exc failures.add("never saw lockIsHeldLatch released"); return; } - assertQ(req("qt", "/admin/threads", "indent", "true") - // monitor owner 'ownerT' - // (which *MAY* also be waiting on doneWithTestLatch, but may not have reached that line yet) - , "//lst[@name='thread'][str[@name='name'][.='test-thread-monitor-owner']]" - + " [arr[@name='monitors-locked']/str[contains(.,'TestMonitorStruct')]]" - ); + + @SuppressWarnings({"unchecked"}) + NamedList<Object> threads = (NamedList<Object>) readProperties()._get("system/threadDump", null); + boolean found = false; + for (Map.Entry<String, Object> threadEntry : threads) { + @SuppressWarnings({"unchecked"}) + NamedList<Object> thread = (NamedList<Object>) threadEntry.getValue(); + // monitor owner 'ownerT' + // (which *MAY* also be waiting on doneWithTestLatch, but may not have reached that line yet) + if (thread._getStr("name", null).contains("test-thread-monitor-owner")) { + if (thread._get("monitors-locked", "").toString().contains("TestMonitorStruct")) { + found = true; + break; + } + } + } + assertTrue(found); Review comment: Please add a failure message to this assertion. ########## File path: solr/core/src/test/org/apache/solr/handler/admin/ThreadDumpHandlerTest.java ########## @@ -214,16 +261,36 @@ public void doTestOwnableSync(final boolean checkWaitingThreadViaPolling) throws Thread.sleep(10); // 10ms at a time, at most 5 sec total } if (lock.hasQueuedThread(blockedT)) { - assertQ(req("qt", "/admin/threads", "indent", "true") - // same lock owner 'ownerT' - , "//lst[@name='thread'][str[@name='name'][.='test-thread-sync-lock-owner']]" - + " [arr[@name='synchronizers-locked']/str[contains(.,'ReentrantLock')]]" - // blocked thread 'blockedT', waiting on the lock - , "//lst[@name='thread'][str[@name='name'][.='test-thread-sync-lock-blocked']]" - + " [str[@name='state'][.='WAITING']]" - + " [lst[@name='lock-waiting'][lst[@name='owner']/str[.='test-thread-sync-lock-owner']]]" - ); - + @SuppressWarnings({"unchecked"}) + NamedList<Object> blockedThreads = (NamedList<Object>) readProperties()._get("system/threadDump", null); + found = false; + for (Map.Entry<String, Object> threadEntry : blockedThreads) { + @SuppressWarnings({"unchecked"}) + NamedList<Object> thread = (NamedList<Object>) threadEntry.getValue(); + // lock owner 'ownerT' + if (thread._getStr("name", null).contains("test-thread-sync-lock-owner")) { + if (thread._get("synchronizers-locked", "").toString().contains("ReentrantLock")) { + found = true; + break; + } + } + } Review comment: Can this logic be refactored into a helper method? It looks like it's been repeated several times already, looking for a thread with a given name and some other condition that could be expressed via a Predicate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org