Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
EndzeitBegins commented on PR #8589: URL: https://github.com/apache/nifi/pull/8589#issuecomment-2036148323 You're welcome. Thank you for the review and merge, @exceptionfactory. -- 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...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
exceptionfactory closed pull request #8589: NIFI-12982 Extend test suite of MockProcessSession URL: https://github.com/apache/nifi/pull/8589 -- 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...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
exceptionfactory commented on code in PR #8589:
URL: https://github.com/apache/nifi/pull/8589#discussion_r1550787557
##
nifi-mock/src/test/java/org/apache/nifi/util/TestMockProcessSession.java:
##
@@ -45,189 +52,588 @@
public class TestMockProcessSession {
-@Test
-public void testReadWithoutCloseThrowsExceptionOnCommit() throws
IOException {
-final Processor processor = new PoorlyBehavedProcessor();
-final MockProcessSession session = new MockProcessSession(new
SharedSessionState(processor, new AtomicLong(0L)), processor, true, new
MockStateManager(processor), true);
-FlowFile flowFile = session.createFlowFile("hello, world".getBytes());
-final InputStream in = session.read(flowFile);
-final byte[] buffer = new byte[12];
-fillBuffer(in, buffer);
+private final Processor processor = new TestProcessor();
+private final SharedSessionState sharedState = new
SharedSessionState(processor, new AtomicLong(0L));
+private final MockStateManager stateManager = new
MockStateManager(processor);
+private final MockProcessSession session = new
MockProcessSession(sharedState, processor, stateManager);
+
+private final Processor statefulProcessor = new StatefulTestProcessor();
+private final SharedSessionState sharedStateOfStatefulProcessor = new
SharedSessionState(statefulProcessor, new AtomicLong(0L));
+private final MockStateManager stateManagerOfStatefulProcessor = new
MockStateManager(statefulProcessor);
+private final MockProcessSession sessionOfStatefulProcessor = new
MockProcessSession(sharedStateOfStatefulProcessor, statefulProcessor,
stateManagerOfStatefulProcessor);
Review Comment:
Thanks for the helpful reply @EndzeitBegins, very good point regarding the
standard per-method behavior. In light of this, and the fact that there are no
Mockito-based mock objects, the current approach makes sense.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
EndzeitBegins commented on code in PR #8589:
URL: https://github.com/apache/nifi/pull/8589#discussion_r1546391808
##
nifi-mock/src/test/java/org/apache/nifi/util/TestMockProcessSession.java:
##
@@ -45,189 +52,588 @@
public class TestMockProcessSession {
-@Test
-public void testReadWithoutCloseThrowsExceptionOnCommit() throws
IOException {
-final Processor processor = new PoorlyBehavedProcessor();
-final MockProcessSession session = new MockProcessSession(new
SharedSessionState(processor, new AtomicLong(0L)), processor, true, new
MockStateManager(processor), true);
-FlowFile flowFile = session.createFlowFile("hello, world".getBytes());
-final InputStream in = session.read(flowFile);
-final byte[] buffer = new byte[12];
-fillBuffer(in, buffer);
+private final Processor processor = new TestProcessor();
+private final SharedSessionState sharedState = new
SharedSessionState(processor, new AtomicLong(0L));
+private final MockStateManager stateManager = new
MockStateManager(processor);
+private final MockProcessSession session = new
MockProcessSession(sharedState, processor, stateManager);
+
+private final Processor statefulProcessor = new StatefulTestProcessor();
+private final SharedSessionState sharedStateOfStatefulProcessor = new
SharedSessionState(statefulProcessor, new AtomicLong(0L));
+private final MockStateManager stateManagerOfStatefulProcessor = new
MockStateManager(statefulProcessor);
+private final MockProcessSession sessionOfStatefulProcessor = new
MockProcessSession(sharedStateOfStatefulProcessor, statefulProcessor,
stateManagerOfStatefulProcessor);
Review Comment:
Thank you for your feedback @exceptionfactory.
I totally agree!
However, as far as I'm aware the default `@TestInstance` lifecycle of JUnit
5 is `PER_METHOD`.
That is, JUnit 5 is creating a new instance of the `TestMockProcessSession`
class for every test method invoked.
As a result, those objects aren't actually reused between test methods.
But there might be some global setting in the NiFi project changing this
default I'm not aware of.
I started out having the "stateful" variants in a single nested scope, but
ended up needing them in another nested scope, as far as I remember, which is
why I moved them up.
I'm fine fine with making these field non final and moving those
instantiations in a `@BeforeEach` if that's preferred. From my perspective
(using mainly Kotlin) it's just more verbose and less immutable.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
EndzeitBegins commented on code in PR #8589:
URL: https://github.com/apache/nifi/pull/8589#discussion_r1546391808
##
nifi-mock/src/test/java/org/apache/nifi/util/TestMockProcessSession.java:
##
@@ -45,189 +52,588 @@
public class TestMockProcessSession {
-@Test
-public void testReadWithoutCloseThrowsExceptionOnCommit() throws
IOException {
-final Processor processor = new PoorlyBehavedProcessor();
-final MockProcessSession session = new MockProcessSession(new
SharedSessionState(processor, new AtomicLong(0L)), processor, true, new
MockStateManager(processor), true);
-FlowFile flowFile = session.createFlowFile("hello, world".getBytes());
-final InputStream in = session.read(flowFile);
-final byte[] buffer = new byte[12];
-fillBuffer(in, buffer);
+private final Processor processor = new TestProcessor();
+private final SharedSessionState sharedState = new
SharedSessionState(processor, new AtomicLong(0L));
+private final MockStateManager stateManager = new
MockStateManager(processor);
+private final MockProcessSession session = new
MockProcessSession(sharedState, processor, stateManager);
+
+private final Processor statefulProcessor = new StatefulTestProcessor();
+private final SharedSessionState sharedStateOfStatefulProcessor = new
SharedSessionState(statefulProcessor, new AtomicLong(0L));
+private final MockStateManager stateManagerOfStatefulProcessor = new
MockStateManager(statefulProcessor);
+private final MockProcessSession sessionOfStatefulProcessor = new
MockProcessSession(sharedStateOfStatefulProcessor, statefulProcessor,
stateManagerOfStatefulProcessor);
Review Comment:
Thank you for your feedback @exceptionfactory.
I totally agree!
However, as far as I'm aware the default `@TestInstance` lifecycle of JUnit
5 is `PER_METHOD`.
That is, JUnit 5 is creating a new instance of the `TestMockProcessSession`
class for every test method invoked.
As a result, those objects aren't actually reused between test methods.
But there might be some global setting in the NiFi project changing this
default I'm not aware of.
I started out having the "stateful" variants in a single nested scope, but
ended up needing them in another nested scope, as far as I remember, which is
why I moved them up.
I'm fine fine with making these field non final and moving those
instantiations in a `@BeforeEach` if that's preferred. From my perspective
(using mainly Kotlin) it's just more verbose.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
EndzeitBegins commented on code in PR #8589:
URL: https://github.com/apache/nifi/pull/8589#discussion_r1546391808
##
nifi-mock/src/test/java/org/apache/nifi/util/TestMockProcessSession.java:
##
@@ -45,189 +52,588 @@
public class TestMockProcessSession {
-@Test
-public void testReadWithoutCloseThrowsExceptionOnCommit() throws
IOException {
-final Processor processor = new PoorlyBehavedProcessor();
-final MockProcessSession session = new MockProcessSession(new
SharedSessionState(processor, new AtomicLong(0L)), processor, true, new
MockStateManager(processor), true);
-FlowFile flowFile = session.createFlowFile("hello, world".getBytes());
-final InputStream in = session.read(flowFile);
-final byte[] buffer = new byte[12];
-fillBuffer(in, buffer);
+private final Processor processor = new TestProcessor();
+private final SharedSessionState sharedState = new
SharedSessionState(processor, new AtomicLong(0L));
+private final MockStateManager stateManager = new
MockStateManager(processor);
+private final MockProcessSession session = new
MockProcessSession(sharedState, processor, stateManager);
+
+private final Processor statefulProcessor = new StatefulTestProcessor();
+private final SharedSessionState sharedStateOfStatefulProcessor = new
SharedSessionState(statefulProcessor, new AtomicLong(0L));
+private final MockStateManager stateManagerOfStatefulProcessor = new
MockStateManager(statefulProcessor);
+private final MockProcessSession sessionOfStatefulProcessor = new
MockProcessSession(sharedStateOfStatefulProcessor, statefulProcessor,
stateManagerOfStatefulProcessor);
Review Comment:
Thank you for your feedback @exceptionfactory.
I totally agree!
However, as far as I'm aware the default `@TestInstance` lifecycle of JUnit
5 is `PER_METHOD`.
That is, JUnit 5 is creating a new instance of the `TestMockProcessSession`
class for every test method invoked.
As a result, those objects aren't actually reused between test methods.
I started out having the "stateful" variants in a single nested scope, but
ended up needing them in another nested scope, as far as I remember, which is
why I moved them up.
I'm fine fine with making these field non final and moving those
instantiations in a `@BeforeEach` if that's preferred. From my perspective
(using mainly Kotlin) it's just more verbose.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12982 Extend test suite of MockProcessSession [nifi]
exceptionfactory commented on code in PR #8589:
URL: https://github.com/apache/nifi/pull/8589#discussion_r1546278329
##
nifi-mock/src/test/java/org/apache/nifi/util/TestMockProcessSession.java:
##
@@ -45,189 +52,588 @@
public class TestMockProcessSession {
-@Test
-public void testReadWithoutCloseThrowsExceptionOnCommit() throws
IOException {
-final Processor processor = new PoorlyBehavedProcessor();
-final MockProcessSession session = new MockProcessSession(new
SharedSessionState(processor, new AtomicLong(0L)), processor, true, new
MockStateManager(processor), true);
-FlowFile flowFile = session.createFlowFile("hello, world".getBytes());
-final InputStream in = session.read(flowFile);
-final byte[] buffer = new byte[12];
-fillBuffer(in, buffer);
+private final Processor processor = new TestProcessor();
+private final SharedSessionState sharedState = new
SharedSessionState(processor, new AtomicLong(0L));
+private final MockStateManager stateManager = new
MockStateManager(processor);
+private final MockProcessSession session = new
MockProcessSession(sharedState, processor, stateManager);
+
+private final Processor statefulProcessor = new StatefulTestProcessor();
+private final SharedSessionState sharedStateOfStatefulProcessor = new
SharedSessionState(statefulProcessor, new AtomicLong(0L));
+private final MockStateManager stateManagerOfStatefulProcessor = new
MockStateManager(statefulProcessor);
+private final MockProcessSession sessionOfStatefulProcessor = new
MockProcessSession(sharedStateOfStatefulProcessor, statefulProcessor,
stateManagerOfStatefulProcessor);
Review Comment:
As a general rule, reusing an instance of an object across test method
invocations can lead to unexpected results.
--
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...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
