SOLR-16696: Breakpoint injection for testing with CommonTestInjection

[patsonluk]

https://issues.apache.org/jira/browse/SOLR-16696

Description

Proposing a new Breakpoint feature in CommonTestInjection which acts similar to breakpoints in IDE.

This allows blocking/checking conditions at the injection point, which could be useful for race condition emulation and exception verification.

Solution

Added a new interface CommonTestInjection$Breakpoint, with a single method executeAndResume. 2 new methods are introduced in CommonTestInjection.

injectBreakpoint(breakpointKey) should be added in the actual code flow guarded by assert, for example assert injectBreakpoint(MyClass.getClass().getName()). assert will make sure the breakpoint logic would only be evaluated during testing.

The actual breakpoint is set by test cases as setBreakpoint(breakpointKey, breakpointImpl), the breakpoint implementation would implement executeAndResume method. Example usages:

Race condition

In the RaceClass instance:

public void racingMethod() {
  //for example a race condition can arise if raceMethod() is invoked and methodA() is called before methodB()
  assert CommonTestInjection.injectBreakpoint(RaceClass.class.getName());
  methodB();
}

in unit test case:

CommonTestInjection.setBreakpoint(RaceClass.class.getName(), () -> methodA());
//...
raceClass.racingMethod();

Caught exception verification

In the target class:

public void myMethod() {
  try {
    //...
  } catch (SomeException e) {
    assert CommonTestInjection.injectBreakpoint("SomeExceptionCheck"); //verify the SomeException was indeed thrown and caught
    //do not re-throw the exception
  }
}

In unit test case:

AtomicBoolean exceptionSeen = new AtomicBoolean(false);
CommonTestInjection.setBreakpoint("SomeExceptionCheck", () -> exceptionSeen.set(true))

instanace.myMethod();
assertTrue(exceptionSeen.get())

Tests

No extra tests are added.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide

[magibney]

This looks really useful -- indeed basically required in order to specifically target certain kinds of race/deadlock issues. Basically identical in spirit to injectDelay(), but more nuanced and reliable.

I left one minor suggestion, but aside from that, I'm wondering if it might be better to introduce this along with the PR that presumably motivated its addition? I think on usefulness it stands in its own right, but it might be clearer in terms of commit history to bundle this as incorporated with a concrete use case.

[magibney]

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.

[patsonluk]

Added with commit 53ee6f8

[patsonluk]

This looks really useful -- indeed basically required in order to specifically target certain kinds of race/deadlock issues. Basically identical in spirit to injectDelay(), but more nuanced and reliable.

I left one minor suggestion, but aside from that, I'm wondering if it might be better to introduce this along with the PR that presumably motivated its addition? I think on usefulness it stands in its own right, but it might be clearer in terms of commit history to bundle this as incorporated with a concrete use case.

Would you mind to review #1460 ? :) that PR actually contains this breakpoint change too and uses it.

I have added extra logging when breakpoint is triggered to that branch instead

[patsonluk]

Closing this as the same change is introduced in #1460