[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 Closed an created a new pull request for the same issue from a clean state: https://github.com/apache/nifi/pull/3165 ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 @MikeThomsen This is kind of tricky: NiFi flows refer the precise version of NiFi within the flow XML file, so you have something like this for the processor definitions: ``` org.apache.nifi.processors.standard.GetHTTP org.apache.nifi nifi-standard-nar 1.7.1 ``` When you export a flow file to test, I would expect the you to use precisely the same NiFi version as the one used to export the flow file. I think there is a little bit of confusion here: please note that these tests do not test the test-harness. The test-harness itself is not tested (kind of a chicken-and-egg issue): these test cases are merely _samples_, which are referenced in the documentation: they demonstrate how an _end-user_ could create test cases for his/her own flows. Please download the ZIP of NiFi 1.7.1 to the `Downloads` directory within your user home (referenced by `org.apache.nifi.testharness.samples.Constants#NIFI_ZIP_DIR` as `new File(System.getProperty("user.home"), "Downloads")` ) and try running the test case again. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2872 > Hi @MikeThomsen sorry for the delay, I have been just very busy. Thanks. I've got a lot on my plate as well, but will try to find some time early this week to review. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 Hi @MikeThomsen sorry for the delay, I have been just very busy. It turns out there have been some changes in the flow XML format, which I have fixed against the latest GA version: now all of the tests are executed properly. I have also introduced strict version checking: the NiFi version referenced in the flow file is checked against the actual version of the NiFi distribution used so that we can avoid confusion in the future. If a user tries to run the test harness with a flow created using a different version of NiFi than the one actually used, this will fail fast, preventing a non functioning installation leaving the user being confused why it does not work properly. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 Hi @MikeThomsen , hi @alopresto , OK, I've implemented my changes according your recommendations: we now have a dedicated profile for running tests, which is disabled by default. (If you decide to make the squashed commit, please use the original `peter-gergely-horvath` as author) ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 That makes sense: I'll look into that... once I have a tiny bit of time... ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/2872 I think there should be a Maven module for the test harness which is disabled by default and can be activated with a flag like `mvn clean test -Ptest-harness`. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2872 @peter-gergely-horvath ok. I'm pretty sure that the surefire plugin can be disabled in the POM, but manually activated, so we'll need to look at that because those tests should be runnable if someone wants to modify the test harness and not roll their own test case. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 Hi @MikeThomsen I _intentionally_ have no test configuration in the project (at least for now): they are merely *samples* of what can be done, but they should not be executed as part of the core NiFi build. NiFi is a beast, starting and stopping it takes some time, I do not want to add that to the each NiFi build. Please create a new Maven project quickstart project (that will have testing configuration enabled) and add the following (replacing `${nifi version}` with the current one) to the dependencies: ``` org.apache.nifi: nifi-testharness ${nifi version} ``` once done, you can take the samples into your own project, where you can experiment with the test harness. I understand piggybacking on the tests directory is maybe not the perfect place to deliver samples, but given the circumstances I think it is acceptable and could maybe be improved in the future. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 @MikeThomsen OK, in the future, I will follow that approach. For this pull request, please review as it is, since the current state contains everything in a squashed commit. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2872 @peter-gergely-horvath the consensus seems to be just keep pushing new commits and let us do a squashed commit for you because that helps GitHub maintain the context of feedback in a review. ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 @MikeThomsen The pull request contains the latest changes; after @lfrancke had made his comments, I corrected the issues mentioned, nuked my whole fork and force-pushed my new version. (I'm wondering what the preferred approach is for you, when there are some suggestions regarding a pull request: maybe it would be worth documenting this in NiFi dev manual?) ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user MikeThomsen commented on the issue: https://github.com/apache/nifi/pull/2872 @peter-gergely-horvath can you push that commit? ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user lfrancke commented on the issue: https://github.com/apache/nifi/pull/2872 Thanks! Unfortunately for some reason I can't see your new commit... (I should maybe also add that I'm not a committer, I wast just interested in this PR) ---
[GitHub] nifi issue #2872: NIFI-5318 Implement NiFi test harness: initial commit of n...
Github user peter-gergely-horvath commented on the issue: https://github.com/apache/nifi/pull/2872 Hi @lfrancke I've fixed the issues you noticed. Can you please review the current state? ---