fgerlits commented on code in PR #1322:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1322#discussion_r866680908
##########
docker/test/integration/environment.py:
##########
@@ -30,7 +34,7 @@ def before_scenario(context, scenario):
return
logging.info("Integration test setup at
{time:%H:%M:%S.%f}".format(time=datetime.datetime.now()))
- context.test = MiNiFi_integration_test(context.image_store)
+ context.test = MiNiFi_integration_test(context.test_id,
context.image_store, context.directory_bindings, context.kind)
Review Comment:
It may be nicer to pass the whole `context` object through all the way from
here `-> MiNiFi_integration_test -> DockerTestCluster ->
SingleNodeDockerCluster` and let classes take what they need from it along the
way.
```suggestion
context.test = MiNiFi_integration_test(context)
```
##########
docker/test/integration/MiNiFi_integration_test_driver.py:
##########
@@ -37,18 +36,17 @@
from minifi.core.utils import decode_escaped_str
-class MiNiFi_integration_test():
- def __init__(self, image_store):
- self.test_id = str(uuid.uuid4())
- self.cluster = DockerTestCluster(image_store)
+class MiNiFi_integration_test:
+ def __init__(self, test_id, image_store, directory_bindings, kind):
Review Comment:
`kind` is not clear here: someone reading the code could think it means
"type".
I think `kind_proxy` would be better. Even better would be to call this
`kubernetes_proxy` and rename `KindProxy` to `KubernetesProxy`, too, as the
fact that we use `kind` is an implementation detail. (Yes, I know I named it
`KindProxy` -- it was a mistake.)
##########
docker/test/integration/MiNiFi_integration_test_driver.py:
##########
@@ -37,18 +36,17 @@
from minifi.core.utils import decode_escaped_str
-class MiNiFi_integration_test():
- def __init__(self, image_store):
- self.test_id = str(uuid.uuid4())
- self.cluster = DockerTestCluster(image_store)
+class MiNiFi_integration_test:
+ def __init__(self, test_id, image_store, directory_bindings, kind):
Review Comment:
`kind` is not clear here: someone reading the code could think it means
"type".
I think `kind_proxy` would be better. Even better would be to call this
`kubernetes_proxy` and rename `KindProxy` to `KubernetesProxy`, too, as the
fact that we use `kind` is an implementation detail. (Yes, I know I named it
`KindProxy` -- it was a mistake.)
##########
docker/test/integration/minifi/core/KindProxy.py:
##########
@@ -93,14 +102,23 @@ def __create_objects_of_type(self, directory, type):
file_name_in_container = os.path.join('/var/tmp', file_name)
self.__copy_file_to_container(full_file_name,
'kind-control-plane', file_name_in_container)
- (code, output) =
self.docker_client.containers.get('kind-control-plane').exec_run(['kubectl',
'apply', '-f', file_name_in_container])
+ (code, _) =
self.docker_client.containers.get('kind-control-plane').exec_run(['kubectl',
'apply', '-f', file_name_in_container])
if code != 0:
raise Exception("Could not create kubernetes object from file
'%s'" % full_file_name)
object_name = file_name.replace(f'.{type}.yml', '')
found_objects.append(object_name)
return found_objects
+ def __delete_objects_of_type(self, type):
+ for full_file_name in
glob.iglob(os.path.join(self.resources_directory, f'*.{type}.yml')):
+ file_name = os.path.basename(full_file_name)
+ file_name_in_container = os.path.join('/var/tmp', file_name)
+
+ (code, _) =
self.docker_client.containers.get('kind-control-plane').exec_run(['kubectl',
'delete', '-f', file_name_in_container, '--grace-period=0', '--force'])
+ if code != 0:
+ raise Exception("Could not delete kubernetes object from file
'%s'" % file_name_in_container)
Review Comment:
We could delete the file in `__create_objects_of_type()`, after `kubectl
apply` finished successfully. Then this function, and some other code which
calls this function, could be removed.
##########
docker/test/integration/environment.py:
##########
@@ -30,7 +34,7 @@ def before_scenario(context, scenario):
return
logging.info("Integration test setup at
{time:%H:%M:%S.%f}".format(time=datetime.datetime.now()))
- context.test = MiNiFi_integration_test(context.image_store)
+ context.test = MiNiFi_integration_test(context.test_id,
context.image_store, context.directory_bindings, context.kind)
Review Comment:
It may be nicer to pass the whole `context` object through all the way from
here `-> MiNiFi_integration_test -> DockerTestCluster ->
SingleNodeDockerCluster` and let classes take what they need from it along the
way.
```suggestion
context.test = MiNiFi_integration_test(context)
```
--
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]
