fgerlits commented on code in PR #2104:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2104#discussion_r2797570379
##########
docker/RunBehaveTests.sh:
##########
@@ -192,22 +192,6 @@ fi
echo "${BEHAVE_OPTS[@]}"
-exec \
- behavex "${BEHAVE_OPTS[@]}" \
- "${docker_dir}/../extensions/standard-processors/tests/features" \
- "${docker_dir}/../extensions/aws/tests/features" \
- "${docker_dir}/../extensions/azure/tests/features" \
- "${docker_dir}/../extensions/sql/tests/features" \
- "${docker_dir}/../extensions/llamacpp/tests/features" \
- "${docker_dir}/../extensions/opc/tests/features" \
- "${docker_dir}/../extensions/kafka/tests/features" \
- "${docker_dir}/../extensions/couchbase/tests/features" \
- "${docker_dir}/../extensions/elasticsearch/tests/features" \
- "${docker_dir}/../extensions/splunk/tests/features" \
- "${docker_dir}/../extensions/gcp/tests/features" \
- "${docker_dir}/../extensions/grafana-loki/tests/features" \
- "${docker_dir}/../extensions/lua/tests/features/" \
- "${docker_dir}/../extensions/civetweb/tests/features/" \
- "${docker_dir}/../extensions/mqtt/tests/features/" \
- "${docker_dir}/../extensions/prometheus/tests/features/" \
- "${docker_dir}/../extensions/python/tests/features/"
+mapfile -t FEATURE_FILES < <(find "${docker_dir}/.." -type f -name '*.feature')
Review Comment:
Do we want to exclude the `docker` directory, or are we going to wait with
merging this until all the old `.feature` files are gone?
We could write
```suggestion
mapfile -t FEATURE_FILES < <(find "${docker_dir}/../extensions" -type f
-name '*.feature')
```
until the old tests are gone.
##########
extensions/aws/tests/features/s3.feature:
##########
@@ -54,7 +54,7 @@ Feature: Sending data from MiNiFi-C++ to an AWS server
And the "failure" relationship of the PutS3Object processor is connected
to the PutS3Object
And PutFile's success relationship is auto-terminated
- And a s3 server is set up in correspondence with the PutS3Object
+ And an s3 server is set up
And the http proxy server is set up
When all instances start up
Review Comment:
Is this correct? Both lines 57 and 59 will call deploy() on the
S3ServerContainer.
##########
extensions/python/tests/features/python.feature:
##########
@@ -84,7 +84,7 @@ Feature: MiNiFi can use python processors in its flows
@USE_NIFI_PYTHON_PROCESSORS_WITH_LANGCHAIN
Scenario Outline: MiNiFi C++ can use native NiFi python processors
Given a GetFile processor with the "Input Directory" property set to
"/tmp/input"
- And a file with filename "test_file.log" and content "test_data" is
present in "/tmp/input"
+ And a directory at "/tmp/input" has a file ("test_file.log") with the
content "test_data"
Review Comment:
minor, but these parentheses look strange, and I think the step would be
clearer without them:
```suggestion
And a directory at "/tmp/input" has a file "test_file.log" with the
content "test_data"
```
##########
extensions/aws/tests/features/steps/steps.py:
##########
@@ -50,10 +50,10 @@ def step_impl(context: MinifiTestContext, processor_name:
str):
context.get_or_create_default_minifi_container().flow_definition.add_processor(processor)
-@step('a s3 server is set up in correspondence with the {processor_name}')
-@step('an s3 server is set up in correspondence with the {processor_name}')
-def step_impl(context: MinifiTestContext, processor_name: str):
+@step('an s3 server is set up')
+def step_impl(context: MinifiTestContext):
context.containers["s3-server"] = S3ServerContainer(context)
+ assert context.containers["s3-server"].deploy()
Review Comment:
I would prefer to leave this step as it is and have a separate "the s3
server starts up" step, to make all the "is set up" steps work in the same way,
and also to make it clear that calling "all instances start up" after this is
an error.
--
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]
