[GitHub] [solr] magibney commented on a diff in pull request #1457: SOLR-16696: Breakpoint injection for testing with CommonTestInjection

["magibney (via GitHub)" <gi...@apache.org>]

magibney commented on code in PR #1457:
URL: https://github.com/apache/solr/pull/1457#discussion_r1321891475


##########
solr/solrj/src/java/org/apache/solr/common/util/CommonTestInjection.java:
##########
@@ -73,4 +76,66 @@ 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)
+   * @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 {@link
+   *     CommonTestInjection#injectBreakpoint(String)}
+   * @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
+   */
+  public static boolean injectBreakpoint(String key) {
+    Breakpoint breakpoint = breakpoints.get(key);
+    if (breakpoint != null) {
+      breakpoint.executeAndResume();
+    }

Review Comment:
   I wonder if we should add logging here, analogous to what's found in `injectDelay()`? The concept is similar, and we have a ready-made "key" to use in the log message.



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org