[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450097829



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   this is a response to a deleted comment:
`text file` is a file format that has the magic number `\n` at the end of 
the file 勞
   
   (I don't know how to resurrect the comment)
   (although I didn't mean to imply that `\n` before EOF -> is text file, e.g. 
having a file with only the magic number won't make a file a zip file)





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450097829



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   this is a response to a deleted comment:
`text file` is a file format that has the magic number `\n` at the end of 
the file 勞
   
   (I don't know how to resurrect the comment)
   (although I didn't mean to imply that '\n' before EOF -> is text file, e.g. 
having a file with only the magic number won't make a file a zip file)





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450097829



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   this is a response to a deleted comment about how `text file` is a file 
format with the magic number `\n` at the end of the file (I don't know how to 
resurrect the comment)





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450073786



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   `text file` is a file format that has the magic number `\n` at the end 
of the file 勞 





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450073786



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   `text-file` is a file format that has the magic number `\n` at the end 
of the file 勞 





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450014693



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   (some more bikeshedding questions about the POSIX "line")
   
   https://user-images.githubusercontent.com/64783590/86563354-85c2ea80-bf64-11ea-8f3a-a5c2ac5f66f8.png;>
   
   what does POSIX say about where the cursor would be? it cannot be on `line 
2` as there is no `line 2`
   
   or take the error message:
   
   https://user-images.githubusercontent.com/64783590/86563648-097cd700-bf65-11ea-87cb-d8d9ebf12ddd.png;>
   
   what is the `2` in `test.cpp:2:8`?
   
   we could have "the line `n`" mean the set of characters after the `(n-1)th 
line` up to the next `\n`, but that seems like a huge stretch





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450014693



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   (some more bikeshedding questions about the POSIX "line")
   
   https://user-images.githubusercontent.com/64783590/86563354-85c2ea80-bf64-11ea-8f3a-a5c2ac5f66f8.png;>
   
   what does POSIX say about where the cursor would be? it cannot be on `line 
2` as there is no `line 2`
   
   or take the error message:
   
   https://user-images.githubusercontent.com/64783590/86563648-097cd700-bf65-11ea-87cb-d8d9ebf12ddd.png;>
   
   what is the `2` in `test.cpp:2:8`?





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450014693



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   (some more bikeshedding questions about the POSIX "line")
   
   https://user-images.githubusercontent.com/64783590/86563354-85c2ea80-bf64-11ea-8f3a-a5c2ac5f66f8.png;>
   
   what does POSIX say about where the cursor would be? it cannot be on "line 
2" as there is no "line 2" 
   
   or take the error message:
   
   https://user-images.githubusercontent.com/64783590/86563648-097cd700-bf65-11ea-87cb-d8d9ebf12ddd.png;>
   
   what is the `2` in `test.cpp:2:8`?





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-06 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r450014693



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   (some more bikeshedding questions about the POSIX "line")
   
   https://user-images.githubusercontent.com/64783590/86563354-85c2ea80-bf64-11ea-8f3a-a5c2ac5f66f8.png;>
   
   what does POSIX say about where the cursor would be? it cannot be on "line 
2" as there is no "line 2" 
   
   or take the error message:
   
   https://user-images.githubusercontent.com/64783590/86563648-097cd700-bf65-11ea-87cb-d8d9ebf12ddd.png;>
   
   what is the `2` in `test.cpp:**2**:8`?





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-01 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r448156824



##
File path: libminifi/test/flow-tests/FlowControllerTests.cpp
##
@@ -0,0 +1,125 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core/Core.h"
+#include "core/repository/AtomicRepoEntries.h"
+#include "core/RepositoryFactory.h"
+#include "FlowFileRecord.h"
+#include "provenance/Provenance.h"
+#include "properties/Configure.h"
+#include "../unit/ProvenanceTestHelper.h"
+#include "../TestBase.h"
+#include "YamlConfiguration.h"
+
+const char* yamlConfig =

Review comment:
   I saw that a number of tests store their config files there, but the 
fact that I have to search for the test by name then check the `CMakeLists.txt` 
to find out what the command line arguments are going to be, and then manually 
navigate to the file, makes me think that it is not the best approach 





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-01 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r448156824



##
File path: libminifi/test/flow-tests/FlowControllerTests.cpp
##
@@ -0,0 +1,125 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NDEBUG
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core/Core.h"
+#include "core/repository/AtomicRepoEntries.h"
+#include "core/RepositoryFactory.h"
+#include "FlowFileRecord.h"
+#include "provenance/Provenance.h"
+#include "properties/Configure.h"
+#include "../unit/ProvenanceTestHelper.h"
+#include "../TestBase.h"
+#include "YamlConfiguration.h"
+
+const char* yamlConfig =

Review comment:
   I saw that a number of tests store their config files there, but the 
fact that I have to search for the test by name then check the CMakeLists.txt 
what the command line arguments are going to be, and then manually navigate to 
the file, makes me think that it is not the best approach 





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-07-01 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r448154480



##
File path: libminifi/src/core/ProcessGroup.cpp
##
@@ -92,13 +92,12 @@ ProcessGroup::~ProcessGroup() {
 onScheduleTimer_->stop();
   }
 
-  for (auto & : connections_) {
+  for (auto&& connection : connections_) {

Review comment:
   I am torn between always use `auto&&` in range-based for loops and 
never, it is really only useful when the iteration yields a temporary proxy 
object like with `std::vector`, `auto&` could be used, I wouldn't use 
`const auto&` as it makes me think that the connection is const whereas only 
the `std::shared_ptr<...>` is





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-06-30 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r447575841



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   (it seems like the Enter puts too much stress on my pinky)





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.

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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #827: MINIFICPP-1273 - Drain connections on flow shutdown

2020-06-30 Thread GitBox


adamdebreceni commented on a change in pull request #827:
URL: https://github.com/apache/nifi-minifi-cpp/pull/827#discussion_r447574816



##
File path: libminifi/test/flow-tests/CMakeLists.txt
##
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+file(GLOB FLOW_TESTS  "*.cpp")
+SET(FLOW_TEST_COUNT 0)
+FOREACH(testfile ${FLOW_TESTS})
+get_filename_component(testfilename "${testfile}" NAME_WE)
+add_executable("${testfilename}" "${testfile}")
+createTests("${testfilename}")
+target_link_libraries(${testfilename} ${CATCH_MAIN_LIB})
+target_wholearchive_library(${testfilename} minifi-standard-processors)
+MATH(EXPR FLOW_TEST_COUNT "${FLOW_TEST_COUNT}+1")
+add_test(NAME "${testfilename}" COMMAND "${testfilename}" 
WORKING_DIRECTORY ${TEST_DIR})
+ENDFOREACH()
+message("-- Finished building ${FLOW_TEST_COUNT} flow related test file(s)...")

Review comment:
   done

##
File path: libminifi/src/core/ProcessGroup.cpp
##
@@ -403,6 +403,16 @@ void 
ProcessGroup::removeConnection(std::shared_ptr connection) {
   }
 }
 
+void ProcessGroup::drainConnections() {
+  for (auto & : connections_) {
+connection->drain();
+  }
+
+  for (std::set::iterator it = child_process_groups_.begin(); 
it != child_process_groups_.end(); ++it) {
+(*it)->drainConnections();
+  }

Review comment:
   done





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.

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