magibney commented on code in PR #1460:
URL: https://github.com/apache/solr/pull/1460#discussion_r1323505098
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -1627,6 +1627,20 @@ private DocCollection fetchCollectionState(String coll,
Watcher watcher)
}
}
return null;
+ } catch (PerReplicaStatesOps.PrsZkNodeNotFoundException e) {
+ CommonTestInjection.injectBreakpoint(ZkStateReader.class.getName() +
"/exercised", e);
Review Comment:
should be invoked as an `assert` statement
##########
solr/solrj/src/java/org/apache/solr/common/util/CommonTestInjection.java:
##########
@@ -73,4 +76,68 @@ public static boolean injectDelay() {
}
return true;
}
+
+ /**
+ * This is usually set by the test cases.
+ *
+ * <p>If defined, code execution would break at certain code execution point
at the invocation of
+ * injectBreakpoint with matching key until the provided method in the
{@link Breakpoint}
+ * implementation is executed.
+ *
+ * <p>Setting the breakpoint to null would remove the breakpoint
+ *
+ * @see CommonTestInjection#injectBreakpoint(String, Object...)
+ * @param key could simply be the fully qualified class name or more
granular like class name +
+ * other id (such as method name). This should batch the key used in
injectBreakpoint
+ * @param breakpoint The Breakpoint implementation, null to remove the
breakpoint
+ */
+ public static void setBreakpoint(String key, Breakpoint breakpoint) {
+ if (breakpoint != null) {
+ breakpoints.put(key, breakpoint);
+ } else {
+ breakpoints.remove(key);
+ }
+ }
+
+ /**
+ * Injects a breakpoint that pauses the existing code execution, executes
the code defined in the
+ * breakpoint implementation and then resumes afterwards. The breakpoint
implementation is looked
+ * up by the corresponding key used in {@link
CommonTestInjection#setBreakpoint(String,
+ * Breakpoint)}
+ *
+ * <p>An example usages :
+ *
+ * <ol>
+ * <li>Inject a precise wait until a race condition is fulfilled before
proceeding with original
+ * code execution
+ * <li>Inject a flag to catch exception statement which handles the
exception without
+ * re-throwing. This could verify caught exception does get triggered
+ * </ol>
+ *
+ * <p>This should always be a part of an assert statement (ie assert
injectBreakpoint(key)) such
+ * that it will be skipped for normal code execution
+ *
+ * @see CommonTestInjection#setBreakpoint(String, Breakpoint)
+ * @param key could simply be the fully qualified class name or more
granular like class name +
+ * other id (such as method name). This should only be set by
corresponding unit test cases
+ * with CommonTestInjection#setBreakpoint
+ * @param args optional arguments list to be passed to the Breakpoint
+ */
+ public static boolean injectBreakpoint(String key, Object... args) {
Review Comment:
The way this is used currently, we're passing a single arg (the exception).
For a utility class like this I guess I'd slightly prefer to have the signature
be a _single_ arg, esp. since the way this will _ever_ be used, afaict, would
involve close coordination between caller and breakpoint-setter. Basically: if
an array needs to be passed, the calling code can just construct an array. I
don't think varargs gets us anything much here. That said, I don't feel too
strongly about it either way.
--
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
