awelless commented on code in PR #10245:
URL: https://github.com/apache/nifi/pull/10245#discussion_r2310381015
##########
nifi-mock/src/main/java/org/apache/nifi/util/MockProcessSession.java:
##########
@@ -285,14 +296,23 @@ public void commit() {
"enabled by calling TestRunner.");
}
- commitInternal();
+ try {
+ commitInternal();
+ } catch (final Throwable t) {
+ rollback();
+ throw t;
+ }
}
private void commitInternal() {
if (!beingProcessed.isEmpty()) {
throw new FlowFileHandlingException("Cannot commit session because
the following FlowFiles have not been removed or transferred: " +
beingProcessed);
}
+ if (shouldFailCommit) {
+ throw new TestFailedCommitException();
Review Comment:
A separate exception class allows us to distinguish between an exception
caused by `failCommit` and an exception caused by illegal flow file handling.
E.g.
[here](https://github.com/apache/nifi/pull/10077/files#diff-60594ac5c47e49ef5a5ce87340edb29188754d494046dbeced840263f87e2d8eR333)
we're waiting specifically for a "scripted" rollback. A thrown
`FlowFileHandlingException` during a test would mean that source code doesn't
handle flow files correctly and should fail the test.
Throwing `FlowFileHandlingException` and catching it in tests might conceal
other failure cases.
##########
nifi-mock/src/main/java/org/apache/nifi/util/MockProcessSession.java:
##########
@@ -1456,4 +1481,73 @@ boolean isFlowFileKnown(final FlowFile flowFile) {
final String providedUuid =
curFlowFile.getAttribute(CoreAttributes.UUID.key());
return curUuid.equals(providedUuid);
}
+
+ public static final class TestFailedCommitException extends
RuntimeException {
+ private TestFailedCommitException() {
+ super("Failing the commit, as requested by the test");
+ }
+ }
+
+ public static final class Builder {
+
+ private final SharedSessionState sharedState;
+ private final Processor processor;
+ private StateManager stateManager;
+
+ private boolean enforceStreamsClosed = true;
+ private boolean allowRecursiveReads = false;
+ private boolean allowSynchronousCommits = false;
+ private boolean shouldFailCommit = false;
Review Comment:
I wanted to make the default values explicit, provided that
`enforseStreamsClosed` is set to `true` by default.
Removed default `false` values.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
