MINIFICPP-2711 Docker tests, missing logs if container fails to start

[martinzink]

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced
  in the commit message?

• Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

• Has your PR been rebased against the latest commit within the target branch (typically main)?

• Is your initial contribution a single, squashed commit?

For code changes:

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file?
• If applicable, have you updated the NOTICE file?

For documentation related changes:

• Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

[Copilot]

Pull request overview

This PR improves the debugging experience for Docker container tests by enabling log output from all containers when a container fails to start. Previously, when a container failed to deploy, only minimal information was available. Now, by passing the MinifiTestContext through to the wait_for_condition helper, the framework can call log_due_to_failure which dumps logs from all containers in the test context, making it much easier to diagnose why a container failed to start.

Changes:

• Updated the Container.deploy() method signature to accept a MinifiTestContext | None parameter
• Updated all container subclass deploy() methods to accept and pass through the context parameter
• Modified test step functions to pass the context when calling deploy()
• Removed redundant or container.log_app_output() calls since logging is now handled by wait_for_condition

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
behave_framework/src/minifi_test_framework/containers/container.py	Base Container class deploy method updated to accept context parameter with TYPE_CHECKING import to avoid circular dependencies
behave_framework/src/minifi_test_framework/containers/minifi_container.py	MiNiFi container deploy method updated to accept and pass through context
behave_framework/src/minifi_test_framework/containers/nifi_container.py	NiFi container deploy method updated to accept and pass through context
behave_framework/src/minifi_test_framework/containers/http_proxy_container.py	HTTP proxy container deploy method updated to accept and pass through context
behave_framework/src/minifi_test_framework/steps/core_steps.py	Test steps updated to pass context to deploy calls and remove redundant log_app_output calls
extensions/aws/tests/features/steps/s3_server_container.py	S3 mock server deploy method updated to accept and pass through context
extensions/aws/tests/features/steps/kinesis_server_container.py	Kinesis mock server deploy method updated to accept and pass through context
extensions/azure/tests/features/steps/azure_server_container.py	Azure storage server deploy method updated to accept and pass through context
extensions/azure/tests/features/steps/steps.py	Azure test steps updated to pass context to deploy
extensions/couchbase/tests/features/steps/couchbase_server_container.py	Couchbase server deploy method updated to accept and pass through context
extensions/couchbase/tests/features/steps/steps.py	Couchbase test steps updated to pass context to deploy
extensions/elasticsearch/tests/features/steps/elastic_base_container.py	Base elastic container deploy method updated to accept context as first parameter
extensions/elasticsearch/tests/features/steps/elasticsearch_container.py	Elasticsearch container deploy method updated to pass context to parent
extensions/elasticsearch/tests/features/steps/opensearch_container.py	Opensearch container deploy method updated to pass context to parent
extensions/elasticsearch/tests/features/steps/steps.py	Elasticsearch test steps updated to pass context to deploy
extensions/gcp/tests/features/steps/fake_gcs_server_container.py	GCS mock server deploy method updated but still passes context=None to wait_for_condition (bug)
extensions/gcp/tests/features/steps/steps.py	GCP test steps updated to pass context to deploy
extensions/grafana-loki/tests/features/steps/grafana_loki_container.py	Grafana Loki container deploy method updated but still passes context=None to wait_for_condition (bug)
extensions/grafana-loki/tests/features/steps/reverse_proxy_container.py	Reverse proxy container deploy method updated to accept and pass through context
extensions/kafka/tests/features/steps/kafka_server_container.py	Kafka server deploy method updated but still passes context=None to wait_for_condition (bug)
extensions/kafka/tests/features/steps/steps.py	Kafka test steps updated to pass context to deploy
extensions/opc/tests/features/steps/opc_ua_server_container.py	OPC UA server deploy method updated to accept and pass through context
extensions/splunk/tests/features/steps/splunk_container.py	Splunk container deploy method updated to accept and pass through context
extensions/splunk/tests/features/steps/steps.py	Splunk test steps updated to pass context to deploy
extensions/sql/tests/features/steps/postgress_server_container.py	PostgreSQL container deploy method updated with import and context parameter
extensions/standard-processors/tests/features/steps/diag_slave_container.py	Modbus diag slave deploy method updated to accept and pass through context
extensions/standard-processors/tests/features/steps/tcp_client_container.py	TCP client container deploy method updated to accept and pass through context
extensions/standard-processors/tests/features/steps/steps.py	Standard processors test steps updated to pass context to deploy

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The type hint uses Union[MinifiTestContext, None] with a forward reference string, while all the subclass implementations use the modern MinifiTestContext | None syntax. For consistency, consider using context: "MinifiTestContext | None" instead of context: "Union[MinifiTestContext, None]". Both are functionally equivalent, but the | syntax is more concise and matches the style used throughout the codebase.

Suggested change

	def deploy(self, context: "Union[MinifiTestContext, None]") -> bool:
	def deploy(self, context: "MinifiTestContext | None") -> bool:

[martinzink]

cant use the modern syntax here due to circular imports

[lordgamez]

What causes circular import with the syntax change?

[martinzink]

With this old syntax the type hint is str and doesnt need actual imports woth thte new syntax it needs the TestContext imported which includes this container file iirc

[lordgamez]

Ah I see, thanks for the explanation. I think that could be solved by using a conditional import with the TYPE_CHECKING constant like it is done in behave_framework/src/minifi_test_framework/core/minifi_test_context.py

[lordgamez]

Is this step needed? I don't see it being used.

[lordgamez]

What causes circular import with the syntax change?