[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1166: MINIFICPP-1331 Upgrade yaml-cpp to eliminate issues with "NULL" string in config

[GitBox <gi...@apache.org>]

fgerlits commented on a change in pull request #1166:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1166#discussion_r703244790



##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -163,10 +163,10 @@ TEST_CASE("Connections components are parsed from yaml", "[YamlConfiguration]")
             "flowfile expiration: \n"
             "drop empty: \n"});
         YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
-        CHECK_THROWS(yaml_connection_parser.getWorkQueueSizeFromYaml());
-        CHECK_THROWS(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
-        CHECK_THROWS(yaml_connection_parser.getFlowFileExpirationFromYaml());
-        CHECK_THROWS(yaml_connection_parser.getDropEmptyFromYaml());
+        REQUIRE(0 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+        REQUIRE(0 == yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+        REQUIRE(0 == yaml_connection_parser.getFlowFileExpirationFromYaml());
+        REQUIRE(0 == yaml_connection_parser.getDropEmptyFromYaml());

Review comment:
       I would leave these as `CHECK(...)`.  In general, `CHECK` is preferable to `REQUIRE`: if eg. the second and fourth tests fail, then with `CHECK`, we will get this info at once, but with `REQUIRE`, we will only know on the first run that the second test failed, then we'll have to fix it and rerun the test to find out that the fourth test fails, too.




-- 
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