szaszm commented on code in PR #2144:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2144#discussion_r2994570265


##########
behave_framework/src/minifi_test_framework/steps/checking_steps.py:
##########
@@ -245,28 +245,28 @@ def step_impl(context: MinifiTestContext, directory: str, 
duration: str, content
 
 
 @then('exactly these files are in the "{directory}" directory in less than 
{duration}: ""')
-def step_impl(context, directory, duration):
+def verify_empty_files_string_in_directory(context, directory, duration):

Review Comment:
   ```suggestion
   def verify_no_files_in_directory(context, directory, duration):
   ```



##########
behave_framework/src/minifi_test_framework/steps/configuration_steps.py:
##########
@@ -21,30 +21,30 @@
 
 
 @step('MiNiFi configuration "{config_key}" is set to "{config_value}"')
-def step_impl(context: MinifiTestContext, config_key: str, config_value: str):
+def set_minifi_config_property(context: MinifiTestContext, config_key: str, 
config_value: str):
     context.get_or_create_default_minifi_container().set_property(config_key, 
config_value)
 
 
 @step("log metrics publisher is enabled in MiNiFi")
-def step_impl(context: MinifiTestContext):
+def enable_minifi_log_metrics_publisher(context: MinifiTestContext):
     
context.get_or_create_default_minifi_container().enable_log_metrics_publisher()
 
 
 @step('log property "{log_property_key}" is set to "{log_property_value}"')
-def step_impl(context: MinifiTestContext, log_property_key: str, 
log_property_value: str):
+def set_minifi_log_property(context: MinifiTestContext, log_property_key: str, 
log_property_value: str):
     
context.get_or_create_default_minifi_container().set_log_property(log_property_key,
 log_property_value)
 
 
 @given("OpenSSL FIPS mode is enabled in MiNiFi")
-def step_impl(context: MinifiTestContext):
+def enable_minifi_openssl_fips_mode(context: MinifiTestContext):
     context.get_or_create_default_minifi_container().enable_openssl_fips_mode()
 
 
 @given("flow configuration path is set up in flow url property")
-def step_impl(context: MinifiTestContext):
+def setup_flow_config_path_in_url(context: MinifiTestContext):

Review Comment:
   To me, the `@given` string, the function name, and the implementation are 
saying 3 slightly different things. I think some clarification about what 
exactly is happening would improve the readability of the code.
   - given: Why do we set the flow config path (conf/config.yml) in the flow 
URL property?
   - fn name: Why do we put the config path in some URL? What URL?
   - body: Fetch the flow configuration from the flow URL (through C2), this is 
clear.



##########
behave_framework/src/minifi_test_framework/steps/core_steps.py:
##########
@@ -72,127 +72,127 @@ def __add_directory_with_file_to_container(context: 
MinifiTestContext, directory
 
 @step('a directory at "{directory}" has a file with the content "{content}" in 
the "{flow_name}" flow')
 @step("a directory at '{directory}' has a file with the content '{content}' in 
the '{flow_name}' flow")
-def step_impl(context: MinifiTestContext, directory: str, content: str, 
flow_name: str):
+def create_file_with_content_in_directory_for_flow(context: MinifiTestContext, 
directory: str, content: str, flow_name: str):
     __add_directory_with_file_to_container(context, directory, 
str(uuid.uuid4()), content, flow_name)
 
 
 @step('a directory at "{directory}" has a file with the content "{content}"')
 @step("a directory at '{directory}' has a file with the content '{content}'")
-def step_impl(context: MinifiTestContext, directory: str, content: str):
+def create_file_with_content_in_directory(context: MinifiTestContext, 
directory: str, content: str):
     context.execute_steps(f'given a directory at "{directory}" has a file with 
the content "{content}" in the "{DEFAULT_MINIFI_CONTAINER_NAME}" flow')
 
 
 @step('a directory at "{directory}" has a file "{file_name}" with the content 
"{content}"')
-def step_impl(context: MinifiTestContext, directory: str, file_name: str, 
content: str):
+def create_file_with_name_and_content_in_directory(context: MinifiTestContext, 
directory: str, file_name: str, content: str):
     __add_directory_with_file_to_container(context, directory, file_name, 
content, DEFAULT_MINIFI_CONTAINER_NAME)
 
 
 @step('a file with filename "{file_name}" and content "{content}" is present 
in "{path}"')
-def step_impl(context: MinifiTestContext, file_name: str, content: str, path: 
str):
+def file_with_name_and_content_present_in_path(context: MinifiTestContext, 
file_name: str, content: str, path: str):
     new_content = content.replace("\\n", "\n")
     
context.get_or_create_default_minifi_container().files.append(File(os.path.join(path,
 file_name), new_content))
 
 
 @given('a file with the content "{content}" is present in "{path}" in the 
"{container_name}" flow')
-def step_impl(context: MinifiTestContext, content: str, path: str, 
container_name: str):
+def file_with_content_present_in_path_for_flow(context: MinifiTestContext, 
content: str, path: str, container_name: str):
     new_content = content.replace("\\n", "\n")
     
context.get_or_create_minifi_container(container_name).files.append(File(os.path.join(path,
 str(uuid.uuid4())), new_content))
 
 
 @given('a file with the content "{content}" is present in "{path}"')
 @given("a file with the content '{content}' is present in '{path}'")
-def step_impl(context: MinifiTestContext, content: str, path: str):
+def file_with_content_present_in_path(context: MinifiTestContext, content: 
str, path: str):
     context.execute_steps(f"given a file with the content \"{content}\" is 
present in \"{path}\" in the \"{DEFAULT_MINIFI_CONTAINER_NAME}\" flow")
 
 
 @when('a file with the content "{content}" is placed in "{path}" in the 
"{container_name}" flow')
-def step_impl(context: MinifiTestContext, content: str, path: str, 
container_name: str):
+def place_file_with_content_in_path_for_flow(context: MinifiTestContext, 
content: str, path: str, container_name: str):
     new_content = content.replace("\\n", "\n")
     
context.containers[container_name].add_file_to_running_container(new_content, 
path)
 
 
 @when('a file with the content "{content}" is placed in "{path}"')
-def step_impl(context: MinifiTestContext, content: str, path: str):
+def place_file_with_content_in_path(context: MinifiTestContext, content: str, 
path: str):
     context.execute_steps(f"when a file with the content \"{content}\" is 
placed in \"{path}\" in the \"{DEFAULT_MINIFI_CONTAINER_NAME}\" flow")
 
 
 @given("an empty file is present in \"{path}\"")
-def step_impl(context, path):
+def empty_file_present_in_path(context, path):
     
context.get_or_create_default_minifi_container().files.append(File(os.path.join(path,
 str(uuid.uuid4())), ""))
 
 
 @given('a host resource file "{filename}" is bound to the "{container_path}" 
path in the MiNiFi container "{container_name}"')
-def step_impl(context: MinifiTestContext, filename: str, container_path: str, 
container_name: str):
+def bind_host_resource_file_to_container_path_for_container(context: 
MinifiTestContext, filename: str, container_path: str, container_name: str):
     path = os.path.join(context.resource_dir, filename)
     context.get_or_create_minifi_container(container_name).add_host_file(path, 
container_path)
 
 
 @given('a host resource file "{filename}" is bound to the "{container_path}" 
path in the MiNiFi container')
-def step_impl(context: MinifiTestContext, filename: str, container_path: str):
+def bind_host_resource_file_to_container_path(context: MinifiTestContext, 
filename: str, container_path: str):
     context.execute_steps(f"given a host resource file \"{filename}\" is bound 
to the \"{container_path}\" path in the MiNiFi container 
\"{DEFAULT_MINIFI_CONTAINER_NAME}\"")
 
 
 @step("after {duration} have passed")
 @step("after {duration} has passed")
-def step_impl(context: MinifiTestContext, duration: str):
+def wait_duration(context: MinifiTestContext, duration: str):
     time.sleep(humanfriendly.parse_timespan(duration))
 
 
 @when("MiNiFi is stopped")
-def step_impl(context: MinifiTestContext):
+def stop_minifi(context: MinifiTestContext):
     context.get_or_create_default_minifi_container().stop()
 
 
 @when("the \"{container_name}\" flow is stopped")
-def step_impl(context: MinifiTestContext, container_name: str):
+def stop_flow(context: MinifiTestContext, container_name: str):
     context.get_or_create_minifi_container(container_name).stop()
 
 
 @when("MiNiFi is restarted")
-def step_impl(context: MinifiTestContext):
+def restart_minifi(context: MinifiTestContext):
     context.get_or_create_default_minifi_container().restart()
 
 
 @when("the \"{container_name}\" flow is restarted")
-def step_impl(context: MinifiTestContext, container_name: str):
+def restart_flow(context: MinifiTestContext, container_name: str):
     context.get_or_create_minifi_container(container_name).restart()
 
 
 @when("the \"{container_name}\" flow is started")
-def step_impl(context: MinifiTestContext, container_name: str):
+def start_flow(context: MinifiTestContext, container_name: str):
     context.get_or_create_minifi_container(container_name).start()
 
 
 @when("the \"{container_name}\" flow is killed")
-def step_impl(context: MinifiTestContext, container_name: str):
+def kill_flow(context: MinifiTestContext, container_name: str):
     context.get_or_create_minifi_container(container_name).kill()
 
 
 @step("the http proxy server is set up")
-def step_impl(context: MinifiTestContext):
+def setup_http_proxy(context: MinifiTestContext):
     context.containers["http-proxy"] = HttpProxy(context)
 
 
 @step("a NiFi container is set up")
-def step_impl(context: MinifiTestContext):
+def setup_nifi_container(context: MinifiTestContext):
     context.containers["nifi"] = NifiContainer(context)
 
 
 @step("a NiFi container is set up with SSL enabled")
-def step_impl(context: MinifiTestContext):
+def setup_nifi_container_with_ssl(context: MinifiTestContext):
     context.containers["nifi"] = NifiContainer(context, use_ssl=True)
 
 
 @when('NiFi is started')
-def step_impl(context):
+def start_nifi(context):
     assert context.containers["nifi"].deploy(context) or 
context.containers["nifi"].log_app_output()
 
 
 @step("{duration} later")
-def step_impl(context: MinifiTestContext, duration: str):
+def wait_duration_later(context: MinifiTestContext, duration: str):

Review Comment:
   This is the same as wait_duration, we should merge them



##########
behave_framework/src/minifi_test_framework/steps/flow_building_steps.py:
##########
@@ -330,67 +330,67 @@ def add_ssl_context_service_for_minifi(context: 
MinifiTestContext, cert_name: st
 
 
 @given("an ssl context service is set up")
-def step_impl(context: MinifiTestContext):
+def setup_ssl_context_service(context: MinifiTestContext):
     add_ssl_context_service_for_minifi(context, "minifi_client")
 
 
 @given("an ssl context service is set up for {processor_name}")
 @given("an ssl context service with a manual CA cert file is set up for 
{processor_name}")
-def step_impl(context, processor_name):
+def setup_ssl_context_service_for_processor(context, processor_name):
     add_ssl_context_service_for_minifi(context, "minifi_client")
     processor = 
context.get_or_create_default_minifi_container().flow_definition.get_processor(processor_name)
     processor.add_property('SSL Context Service', 'SSLContextService')
 
 
 @given("an ssl context service using the system CA cert store is set up for 
{processor_name}")
-def step_impl(context: MinifiTestContext, processor_name):
+def setup_ssl_context_service_system_ca_for_processor(context: 
MinifiTestContext, processor_name):
     add_ssl_context_service_for_minifi(context, "minifi_client", 
use_system_cert_store=True)
     processor = 
context.get_or_create_default_minifi_container().flow_definition.get_processor(processor_name)
     processor.add_property('SSL Context Service', 'SSLContextService')
 
 
 @given("a RemoteProcessGroup node with name \"{rpg_name}\" is opened on 
\"{address}\" with transport protocol set to \"{transport_protocol}\"")
-def step_impl(context: MinifiTestContext, rpg_name: str, address: str, 
transport_protocol: str):
+def open_rpg_with_transport_protocol(context: MinifiTestContext, rpg_name: 
str, address: str, transport_protocol: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_remote_process_group(address,
 rpg_name, transport_protocol)
 
 
 @given("a RemoteProcessGroup node with name \"{rpg_name}\" is opened on 
\"{address}\"")
-def step_impl(context: MinifiTestContext, rpg_name: str, address: str):
+def open_rpg(context: MinifiTestContext, rpg_name: str, address: str):
     context.execute_steps(f"given a RemoteProcessGroup node with name 
\"{rpg_name}\" is opened on \"{address}\" with transport protocol set to 
\"RAW\"")
 
 
 @given("an input port with name \"{port_name}\" is created on the 
RemoteProcessGroup named \"{rpg_name}\"")
-def step_impl(context: MinifiTestContext, port_name: str, rpg_name: str):
+def create_input_port_on_rpg(context: MinifiTestContext, port_name: str, 
rpg_name: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_input_port_to_rpg(rpg_name,
 port_name)
 
 
 @given("an input port using compression with name \"{port_name}\" is created 
on the RemoteProcessGroup named \"{rpg_name}\"")
-def step_impl(context: MinifiTestContext, port_name: str, rpg_name: str):
+def create_input_port_with_compression_on_rpg(context: MinifiTestContext, 
port_name: str, rpg_name: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_input_port_to_rpg(rpg_name,
 port_name, use_compression=True)
 
 
 @given("an output port with name \"{port_name}\" is created on the 
RemoteProcessGroup named \"{rpg_name}\"")
-def step_impl(context: MinifiTestContext, port_name: str, rpg_name: str):
+def create_output_port_on_rpg(context: MinifiTestContext, port_name: str, 
rpg_name: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_output_port_to_rpg(rpg_name,
 port_name)
 
 
 @given("an output port using compression with name \"{port_name}\" is created 
on the RemoteProcessGroup named \"{rpg_name}\"")
-def step_impl(context: MinifiTestContext, port_name: str, rpg_name: str):
+def create_output_port_with_compression_on_rpg(context: MinifiTestContext, 
port_name: str, rpg_name: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_output_port_to_rpg(rpg_name,
 port_name, use_compression=True)
 
 
 @given("a NiFi flow is receiving data from the RemoteProcessGroup named 
\"{rpg_name}\" in an input port named \"{input_port_name}\" which has the same 
id as the port named \"{rpg_port_name}\"")
-def step_impl(context: MinifiTestContext, input_port_name: str, rpg_port_name: 
str, rpg_name: str):
+def nifi_receive_from_rpg_input_port(context: MinifiTestContext, 
input_port_name: str, rpg_port_name: str, rpg_name: str):
     input_port_id = 
context.get_or_create_default_minifi_container().flow_definition.get_input_port_id_of_rpg(rpg_name,
 rpg_port_name)
     context.containers["nifi"].flow_definition.add_input_port(input_port_id, 
input_port_name)
 
 
 @given("a NiFi flow is sending data to an output port named \"{port_name}\" 
with the id of the port named \"{rpg_port_name}\" from the RemoteProcessGroup 
named \"{rpg_name}\"")
-def step_impl(context: MinifiTestContext, port_name: str, rpg_port_name: str, 
rpg_name: str):
+def nifi_send_to_rpg_output_port(context: MinifiTestContext, port_name: str, 
rpg_port_name: str, rpg_name: str):
     output_port_id = 
context.get_or_create_default_minifi_container().flow_definition.get_output_port_id_of_rpg(rpg_name,
 rpg_port_name)
     context.containers["nifi"].flow_definition.add_output_port(output_port_id, 
port_name)
 
 
 @given("the connection going to {destination} has \"drop empty\" set")
-def step_impl(context: MinifiTestContext, destination: str):
+def set_drop_empty_for_connection(context: MinifiTestContext, destination: 
str):

Review Comment:
   ```suggestion
   def set_drop_empty_flag_for_connection(context: MinifiTestContext, 
destination: str):
   ```



##########
behave_framework/src/minifi_test_framework/steps/flow_building_steps.py:
##########
@@ -143,106 +143,106 @@ def step_impl(context: MinifiTestContext, 
property_name: str, controller_name: s
 
 
 @step('a Funnel with the name "{funnel_name}" is set up')
-def step_impl(context: MinifiTestContext, funnel_name: str):
+def setup_funnel(context: MinifiTestContext, funnel_name: str):
     
context.get_or_create_default_minifi_container().flow_definition.add_funnel(Funnel(funnel_name))
 
 
 @step('in the "{minifi_container_name}" flow the "{relationship_name}" 
relationship of the {source} processor is connected to the {target}')
 @step('in the "{minifi_container_name}" flow the "{relationship_name}" 
relationship of the {source} node is connected to the {target}')
-def step_impl(context: MinifiTestContext, relationship_name: str, source: str, 
target: str, minifi_container_name: str):
+def connect_relationship_in_minifi_flow(context: MinifiTestContext, 
relationship_name: str, source: str, target: str, minifi_container_name: str):
     connection = Connection(source_name=source, 
source_relationship=relationship_name, target_name=target)
     
context.get_or_create_minifi_container(minifi_container_name).flow_definition.add_connection(connection)
 
 
 @step('in the NiFi flow the "{relationship_name}" relationship of the {source} 
processor is connected to the {target}')
-def step_impl(context: MinifiTestContext, relationship_name: str, source: str, 
target: str):
+def connect_relationship_in_nifi_flow(context: MinifiTestContext, 
relationship_name: str, source: str, target: str):
     connection = Connection(source_name=source, 
source_relationship=relationship_name, target_name=target)
     context.containers["nifi"].flow_definition.add_connection(connection)
 
 
 @step('the "{relationship_name}" relationship of the {source} processor is 
connected to the {target}')
 @step('the "{relationship_name}" relationship of the {source} node is 
connected to the {target}')
-def step_impl(context: MinifiTestContext, relationship_name: str, source: str, 
target: str):
+def connect_relationship(context: MinifiTestContext, relationship_name: str, 
source: str, target: str):
     context.execute_steps(f'given in the "{DEFAULT_MINIFI_CONTAINER_NAME}" 
flow the "{relationship_name}" relationship of the {source} processor is 
connected to the {target}')
 
 
 @step("the output port \"{port_name}\" is connected to the {destination_name} 
processor")
-def step_impl(context: MinifiTestContext, port_name: str, destination_name: 
str):
+def connect_output_port_to_processor(context: MinifiTestContext, port_name: 
str, destination_name: str):
     connection = Connection(source_name=port_name, 
source_relationship="undefined", target_name=destination_name)
     
context.get_or_create_minifi_container(DEFAULT_MINIFI_CONTAINER_NAME).flow_definition.add_connection(connection)
 
 
 @step('the Funnel with the name "{funnel_name}" is connected to the {target}')
-def step_impl(context: MinifiTestContext, funnel_name: str, target: str):
+def connect_funnel_to_target(context: MinifiTestContext, funnel_name: str, 
target: str):
     connection = Connection(source_name=funnel_name, 
source_relationship="success", target_name=target)
     
context.get_or_create_default_minifi_container().flow_definition.add_connection(connection)
 
 
 @step("{processor_name}'s {relationship} relationship is auto-terminated in 
the \"{minifi_container_name}\" flow")
-def step_impl(context: MinifiTestContext, processor_name: str, relationship: 
str, minifi_container_name: str):
+def auto_terminate_relationship_in_minifi_flow(context: MinifiTestContext, 
processor_name: str, relationship: str, minifi_container_name: str):
     
context.get_or_create_minifi_container(minifi_container_name).flow_definition.get_processor(processor_name).auto_terminated_relationships.append(
         relationship)
 
 
 @step("{processor_name}'s {relationship} relationship is auto-terminated in 
the NiFi flow")
-def step_impl(context: MinifiTestContext, processor_name: str, relationship: 
str):
+def auto_terminate_relationship_in_nifi_flow(context: MinifiTestContext, 
processor_name: str, relationship: str):
     
context.containers["nifi"].flow_definition.get_processor(processor_name).auto_terminated_relationships.append(relationship)
 
 
 @step("{processor_name}'s {relationship} relationship is auto-terminated")
 @step("the \"{relationship}\" relationship of the {processor_name} processor 
is auto-terminated")
-def step_impl(context: MinifiTestContext, processor_name: str, relationship: 
str):
+def auto_terminate_relationship(context: MinifiTestContext, processor_name: 
str, relationship: str):
     context.execute_steps(f'given {processor_name}\'s {relationship} 
relationship is auto-terminated in the "{DEFAULT_MINIFI_CONTAINER_NAME}" flow')
 
 
 @given("a transient MiNiFi flow is set up")
-def step_impl(context: MinifiTestContext):
+def setup_transient_minifi_flow(context: MinifiTestContext):
     context.get_or_create_default_minifi_container().command = ["/bin/sh", 
"-c", "timeout 10s ./bin/minifi.sh run && sleep 100"]
 
 
 @step('the scheduling period of the {processor_name} processor is set to 
"{duration_str}" in the "{minifi_container_name}" flow')
-def step_impl(context: MinifiTestContext, processor_name: str, duration_str: 
str, minifi_container_name: str):
+def set_scheduling_period_in_minifi_flow(context: MinifiTestContext, 
processor_name: str, duration_str: str, minifi_container_name: str):
     
context.get_or_create_minifi_container(minifi_container_name).flow_definition.get_processor(processor_name).scheduling_period
 = duration_str
 
 
 @step('the scheduling period of the {processor_name} processor is set to 
"{duration_str}" in the NiFi flow')
-def step_impl(context: MinifiTestContext, processor_name: str, duration_str: 
str, minifi_container_name: str):
+def set_scheduling_period_in_nifi_flow(context: MinifiTestContext, 
processor_name: str, duration_str: str):
     
context.containers["nifi"].flow_definition.get_processor(processor_name).scheduling_period
 = duration_str
 
 
 @step('the scheduling period of the {processor_name} processor is set to 
"{duration_str}"')
-def step_impl(context: MinifiTestContext, processor_name: str, duration_str: 
str):
+def set_scheduling_period(context: MinifiTestContext, processor_name: str, 
duration_str: str):
     
context.get_or_create_default_minifi_container().flow_definition.get_processor(processor_name).scheduling_period
 = duration_str
 
 
 @given("parameter context name is set to '{context_name}'")
-def step_impl(context: MinifiTestContext, context_name: str):
+def set_parameter_context_name(context: MinifiTestContext, context_name: str):

Review Comment:
   The function impl adds a new parameter context, I think the `@given` string 
and the function name are misleading.
   ```suggestion
   def add_parameter_context(context: MinifiTestContext, context_name: str):
   ```



##########
extensions/standard-processors/tests/features/steps/steps.py:
##########
@@ -31,33 +31,33 @@
 
 
 @step("a Syslog client with TCP protocol is setup to send logs to minifi")
-def step_impl(context: MinifiTestContext):
+def setup_syslog_tcp_client(context: MinifiTestContext):
     context.containers["syslog-tcp"] = SyslogContainer("tcp", context)
 
 
 @step("a Syslog client with UDP protocol is setup to send logs to minifi")
-def step_impl(context: MinifiTestContext):
+def setup_syslog_udp_client(context: MinifiTestContext):
     context.containers["syslog-udp"] = SyslogContainer("udp", context)
 
 
 @step('there is an accessible PLC with modbus enabled')
-def step_impl(context: MinifiTestContext):
+def setup_accessible_plc_with_modbus(context: MinifiTestContext):

Review Comment:
   ```suggestion
   def setup_plc_with_modbus(context: MinifiTestContext):
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to