You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@nifi.apache.org by "markap14 (via GitHub)" <gi...@apache.org> on 2026/08/29 16:30:31 UTC

[PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

markap14 opened a new pull request, #11599:
URL: https://github.com/apache/nifi/pull/11599

   ## Summary
   - Adds a new `nifi-system-test-framework` module containing `NiFiSystemIT` and its supporting infrastructure (`NiFiInstance`, `InstanceConfiguration`, `SpawnedStandaloneNiFiInstanceFactory`, `SpawnedClusterNiFiInstanceFactory`, `AggregateNiFiInstance`, `NiFiInstanceCache`, `NiFiClientUtil`, `NiFiSystemKeyStoreProvider`, `TroubleshootingTestWatcher`, etc.), moved to `src/main/java` so the artifact is a normal, publishable dependency rather than test-scoped code trapped inside `nifi-system-test-suite`.
   - Third-party NAR projects can now depend on `nifi-system-test-framework` (as a `test`-scope dependency, the same way `nifi-mock` is depended on for unit tests) to write their own System Tests that spawn a real NiFi instance and drive it via the REST client, without needing a full NiFi source checkout.
   - `nifi-system-test-suite` now depends on the new module instead of defining these classes itself. It retains all of its own `*IT` test classes, test resources, and the NiFi-specific `AbstractNarSwapMigrationIT` base class (which is tied to internal test-only NARs and isn't generic).
   - No behavior changes: package names are unchanged (`org.apache.nifi.tests.system`), so none of the ~500 existing `*IT` test classes required edits.
   
   ## Test plan
   - [x] `mvn -pl nifi-system-tests/nifi-system-test-framework -am install -DskipTests` — new module compiles and installs.
   - [x] `mvn -pl nifi-system-tests/nifi-system-test-suite -am install -DskipTests` — suite compiles against the new dependency.
   - [x] `mvn -pl nifi-system-tests/nifi-system-test-suite verify -Pintegration-tests,skip-unit-tests -Dit.test=ClassloaderIsolationKeyIT` — ran a real System Test end-to-end (spawns a NiFi instance) using `NiFiSystemIT` from the extracted module: `Tests run: 2, Failures: 0, Errors: 0`.


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 closed pull request #11599: NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite
URL: https://github.com/apache/nifi/pull/11599


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5481419646

   @exceptionfactory I'm not sure that I would call this "a module intended for extension." This is a module intended to start up 1 nifi instance or one full cluster of nifi instances in a light-weight manner purely with the goal of verifying behavior against a legit, running instance for automated tests. I don't think we can remove things like Jersey or the Framework Cluster Protocol because doing so would fail to run NiFi as it's run in production. Please correct me if I'm misunderstanding something.


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.

exceptionfactory commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5481504220

   By module for extension, I was referring specifically to `NiFiSystemIT` as a base test class that others would extend. The other remaining `public` classes also imply the need for reuse. That's based on the usage in existing system tests. Is there a different way that system test builders would use this module?
   
   At the root, I'm concerned about the lack of encapsulation in the current approach. The design has worked well when everything was co-located in the system test suite, but it seems like just moving out these classes doesn't provide the kind of approach needed for test interfaces versus test implementation.
   
   Taking a different perspective, it also raises a question as to whether a Testcontainer approach might be better, as that would encapsulate the entire application, without these types of module contract concerns. I realize this is a significantly different path, but it seems worth considering as an alternative.


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.

exceptionfactory commented on code in PR #11599:
URL: https://github.com/apache/nifi/pull/11599#discussion_r3905434023


##########
nifi-system-tests/nifi-system-test-framework/src/main/java/org/apache/nifi/tests/system/NiFiSystemIT.java:
##########
@@ -94,6 +92,27 @@ public abstract class NiFiSystemIT implements NiFiInstanceProvider {
     public static final String TEST_REPORTING_TASK_PACKAGE = "org.apache.nifi.reporting";
     public static final String TEST_FLOW_ANALYSIS_RULE_PACKAGE = "org.apache.nifi.flowanalysis";
 
+    // Cluster role/node-state values as reported by the REST API's NodeDTO. Defined here as plain Strings, matching
+    // the wire format, instead of depending on nifi-framework-cluster-protocol's ClusterRoles/NodeConnectionState.
+    public static final String CLUSTER_ROLE_PRIMARY_NODE = "Primary Node";
+    public static final String CLUSTER_ROLE_COORDINATOR = "Cluster Coordinator";
+    public static final String NODE_STATE_CONNECTING = "CONNECTING";
+    public static final String NODE_STATE_CONNECTED = "CONNECTED";
+    public static final String NODE_STATE_OFFLOADING = "OFFLOADING";
+    public static final String NODE_STATE_DISCONNECTING = "DISCONNECTING";
+    public static final String NODE_STATE_DISCONNECTED = "DISCONNECTED";

Review Comment:
   Can these be changed to use `NodeConnectionState` which is now part of `nifi-api`?



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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5485373796

Ahh I see what you're saying now @exceptionfactory. I don't think Testcontainers is really the way we want to go here because that would require that the full docker image be built so it could be usable. That would absolutely make sense for testing extensions such as Processors in a separate service (by uploading the nar, etc). But it wouldn't work for framework-level extensions.

That said, I reviewed the pom.xml again and I think I see the concern. With the latest commit we now have basically only 2 sets of dependencies:
- Those on the nifi root lib/ directory (these are necessary) to start the nifi instance
- The nifi-toolkit-client necessary for communicating with the nifi instance. This includes the jersey dependency. An argument can definitely be made that it's actually a bug in the toolkit-client code that it's not explicitly defined there, but it's necessary at runtime.

The pom that I had factored out did have a few extra dependencies that I was able to eliminate with some simple refactoring:
- nifi-per-process-group-logging (not actually needed in the 'framework' module, just the test that uses it)
- commons-lang3, nifi-xml-processing (not actually necessary for the 'framework' module either)
- nifi-framework-cluster-protocol (used by the framework module but it was just for access to enums to compare to DTO String values, etc. so I was able to do some very simple minor refactoring to eliminate the dependency).

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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on code in PR #11599:
URL: https://github.com/apache/nifi/pull/11599#discussion_r3905773560


##########
nifi-system-tests/nifi-system-test-framework/src/main/java/org/apache/nifi/tests/system/NiFiSystemIT.java:
##########
@@ -94,6 +92,27 @@ public abstract class NiFiSystemIT implements NiFiInstanceProvider {
     public static final String TEST_REPORTING_TASK_PACKAGE = "org.apache.nifi.reporting";
     public static final String TEST_FLOW_ANALYSIS_RULE_PACKAGE = "org.apache.nifi.flowanalysis";
 
+    // Cluster role/node-state values as reported by the REST API's NodeDTO. Defined here as plain Strings, matching
+    // the wire format, instead of depending on nifi-framework-cluster-protocol's ClusterRoles/NodeConnectionState.
+    public static final String CLUSTER_ROLE_PRIMARY_NODE = "Primary Node";
+    public static final String CLUSTER_ROLE_COORDINATOR = "Cluster Coordinator";
+    public static final String NODE_STATE_CONNECTING = "CONNECTING";
+    public static final String NODE_STATE_CONNECTED = "CONNECTED";
+    public static final String NODE_STATE_OFFLOADING = "OFFLOADING";
+    public static final String NODE_STATE_DISCONNECTING = "DISCONNECTING";
+    public static final String NODE_STATE_DISCONNECTED = "DISCONNECTED";

Review Comment:
   Ah yeah, that's a great call! Just pushed an update



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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5470752299

   Good catch, thanks @exceptionfactory. I went through the moved classes and checked (via grep across all ~500 existing \`*IT\` test classes, which live in subpackages of \`org.apache.nifi.tests.system\`) which ones are actually referenced from outside the framework's own package.
   
   \`NiFiInstanceProvider\`, \`AggregateNiFiInstance\`, \`NiFiInstanceCache\`, \`NiFiSystemKeyStoreProvider\`, and \`TroubleshootingTestWatcher\` (including the one you called out) have no external consumers - they're only used internally by \`NiFiSystemIT\` and its factory implementations, so I made them package-private in the latest commit.
   
   \`NiFiSystemIT\`, \`NiFiInstance\`, \`NiFiInstanceFactory\`, \`InstanceConfiguration\`, \`SpawnedStandaloneNiFiInstanceFactory\`, \`SpawnedClusterNiFiInstanceFactory\`, \`NiFiClientUtil\`, and \`ExceptionalBooleanSupplier\` do need to stay public - they're directly extended/constructed/imported from test classes in other packages today.
   
   Rebuilt and reran the sample IT test to confirm nothing broke.


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5497741926

   That's a great catch @exceptionfactory. I admitted haven't considered all of the conf/ files. At this point I think I'm going to close this PR for now because while I think this is valuable, my expectation was that this was going to be a very simple refactor to create a pom.xml or two and move some classes. A few minor refactors was reasonable. But at this point I think the scope is growing to the point that this would need to be more adequately planned instead of simple adhoc adjustments. We can definitely return to this in the future as time permits.


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

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

Re: [PR] NIFI-16265 - Extract reusable System Test framework from nifi-system-test-suite [nifi]

Posted by "markap14 (via GitHub)" <gi...@apache.org>.

markap14 commented on PR #11599:
URL: https://github.com/apache/nifi/pull/11599#issuecomment-5470724432

   That's fair @exceptionfactory. My intent was to avoid changing anything and instead move everything over exactly as-is. But you're right, moving from test to publicly available it is probably fair to reduce visibility on some of those.


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

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