exceptionfactory commented on code in PR #10245:
URL: https://github.com/apache/nifi/pull/10245#discussion_r2310440691
##########
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:
That is true that using `FlowFileHandlingException` could be confused with
other exceptions. On the other hand, introducing a new Exception class exposes
expands the API surface of `nifi-mock`.
The `StandardProcessSession` can throw several different kinds of
exceptions, including `FlowFileAccessException`, and of course incorrect
behavior in a Processor can lead to other exceptions. An `UncheckedIOException`
could be another option. I think it is better than any test code expect a
generic `RuntimeException`, as opposed to a specific exception class, as other
test methods can evaluate correct behavior.
--
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]
