SDK - Tests - Fixed most of the test warnings (#2336)

sdk/python/kfp/aws.py

@@ -30,8 +30,8 @@ def use_aws_secret(secret_name='aws-secret', aws_access_key_id_name='AWS_ACCESS_
 
     def _use_aws_secret(task):
         from kubernetes import client as k8s_client
-        return (
-            task
+        (
+            task.container
                 .add_env_variable(
                     k8s_client.V1EnvVar(
                         name='AWS_ACCESS_KEY_ID',
@@ -55,5 +55,6 @@ def _use_aws_secret(task):
                     )
                 )
         )
+        return task
 
     return _use_aws_secret

sdk/python/kfp/azure.py

@@ -24,8 +24,8 @@ def use_azure_secret(secret_name='azcreds'):
 
     def _use_azure_secret(task):
         from kubernetes import client as k8s_client
-        return (
-            task            
+        (
+            task.container
                 .add_env_variable(
                     k8s_client.V1EnvVar(
                         name='AZ_SUBSCRIPTION_ID',
@@ -71,5 +71,6 @@ def _use_azure_secret(task):
                     )
                 )
         )
+        return task
 
     return _use_azure_secret

sdk/python/tests/compiler/testdata/withitem_basic.py

@@ -29,7 +29,7 @@ def get_code(self, ):
 
 
 @dsl.pipeline(name='my-pipeline')
-def pipeline(my_pipe_param=10):
+def pipeline(my_pipe_param: int = 10):
     loop_args = [{'a': 1, 'b': 2}, {'a': 10, 'b': 20}]
     with dsl.ParallelFor(loop_args) as item:
         op1 = dsl.ContainerOp(

sdk/python/tests/compiler/testdata/withitem_basic.yaml

@@ -2,7 +2,7 @@ apiVersion: argoproj.io/v1alpha1
 kind: Workflow
 metadata:
   annotations:
-    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "10", "name": "my_pipe_param"}],
+    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "10", "name": "my_pipe_param", "type": "Integer"}],
       "name": "my-pipeline"}'
   generateName: my-pipeline-
 spec:

sdk/python/tests/compiler/testdata/withitem_nested.py

@@ -29,7 +29,7 @@ def get_code(self, ):
 
 
 @dsl.pipeline(name='my-pipeline')
-def pipeline(my_pipe_param=10):
+def pipeline(my_pipe_param: int = 10):
     loop_args = [{'a': 1, 'b': 2}, {'a': 10, 'b': 20}]
     with dsl.ParallelFor(loop_args) as item:
         op1 = dsl.ContainerOp(

sdk/python/tests/compiler/testdata/withitem_nested.yaml

@@ -2,7 +2,7 @@ apiVersion: argoproj.io/v1alpha1
 kind: Workflow
 metadata:
   annotations:
-    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "10", "name": "my_pipe_param"}],
+    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "10", "name": "my_pipe_param", "type": "Integer"}],
       "name": "my-pipeline"}'
   generateName: my-pipeline-
 spec:

sdk/python/tests/compiler/testdata/withparam_global.py

@@ -29,7 +29,7 @@ def get_code(self, ):
 
 
 @dsl.pipeline(name='my-pipeline')
-def pipeline(loopidy_doop=[3, 5, 7, 9]):
+def pipeline(loopidy_doop: list = [3, 5, 7, 9]):
     op0 = dsl.ContainerOp(
         name="my-out-cop0",
         image='python:alpine3.6',

sdk/python/tests/compiler/testdata/withparam_global.yaml

@@ -3,7 +3,7 @@ kind: Workflow
 metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "[3, 5, 7, 9]", "name":
-      "loopidy_doop"}], "name": "my-pipeline"}'
+      "loopidy_doop", "type": "JsonArray"}], "name": "my-pipeline"}'
   generateName: my-pipeline-
 spec:
   arguments:

sdk/python/tests/compiler/testdata/withparam_global_dict.py

@@ -29,7 +29,7 @@ def get_code(self, ):
 
 
 @dsl.pipeline(name='my-pipeline')
-def pipeline(loopidy_doop=[{'a': 1, 'b': 2}, {'a': 10, 'b': 20}]):
+def pipeline(loopidy_doop: dict = [{'a': 1, 'b': 2}, {'a': 10, 'b': 20}]):
     op0 = dsl.ContainerOp(
         name="my-out-cop0",
         image='python:alpine3.6',

sdk/python/tests/compiler/testdata/withparam_global_dict.yaml

@@ -3,7 +3,7 @@ kind: Workflow
 metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "[{\"a\": 1, \"b\":
-      2}, {\"a\": 10, \"b\": 20}]", "name": "loopidy_doop"}], "name": "my-pipeline"}'
+      2}, {\"a\": 10, \"b\": 20}]", "name": "loopidy_doop", "type": "JsonObject"}], "name": "my-pipeline"}'
   generateName: my-pipeline-
 spec:
   arguments:

sdk/python/tests/components/test_components.py

@@ -532,8 +532,8 @@ def test_command_if_is_present_then_else(self):
     def test_command_if_input_value_then(self):
         component_text = '''\
 inputs:
-- {name: Do test, type: boolean, optional: true}
-- {name: Test data, optional: true}
+- {name: Do test, type: Boolean, optional: true}
+- {name: Test data, type: Integer, optional: true}
 - {name: Test parameter 1, optional: true}
 implementation:
   container:
@@ -545,7 +545,7 @@ def test_command_if_input_value_then(self):
 '''
         task_factory1 = comp.load_component(text=component_text)
 
-        task_then = task_factory1(True, 'test_data.txt', 42)
+        task_then = task_factory1(True, 'test_data.txt', '42')
         self.assertEqual(task_then.arguments, ['--test-data', 'test_data.txt', '--test-param1', '42'])
 
         task_else = task_factory1()

sdk/python/tests/components/test_graph_components.py

@@ -199,7 +199,7 @@ def test_load_graph_component(self):
                 image: busybox
                 command: [sh, -c, 'echo "$0" > $1; echo "$0" > $2', {inputValue: in1_1}, {outputPath: out1_1}, {outputPath: out1_2}]
         arguments:
-            in1_1: 11
+            in1_1: '11'
       task 2:
         componentRef:
           spec:
@@ -214,7 +214,7 @@ def test_load_graph_component(self):
                 image: busybox
                 command: [sh, -c, 'cat "$0" "$1" > $2', {inputValue: in2_1}, {inputValue: in2_2}, {outputPath: out2_1}]
         arguments:
-            in2_1: 21
+            in2_1: '21'
             in2_2: {taskOutput: {taskId: task 1, outputName: out1_1}}
       task 3:
         componentRef:
@@ -236,7 +236,7 @@ def test_load_graph_component(self):
       graph out 1: {taskOutput: {taskId: task 3, outputName: out3_1}}
       graph out 2: {taskOutput: {taskId: task 1, outputName: out1_2}}
       graph out 3: {graphInput: graph in 2}
-      graph out 4: 42
+      graph out 4: '42'
 '''
         op = comp.load_component_from_text(component_text)
         task = op('graph 1', 'graph 2')
@@ -269,7 +269,7 @@ def test_load_nested_graph_components(self):
                 image: busybox
                 command: [sh, -c, 'echo "$0" > $1; echo "$0" > $2', {inputValue: in1_1}, {outputPath: out1_1}, {outputPath: out1_2}]
         arguments:
-            in1_1: 11
+            in1_1: '11'
       task 2:
         componentRef:
           spec:
@@ -284,7 +284,7 @@ def test_load_nested_graph_components(self):
                 image: busybox
                 command: [sh, -c, 'cat "$0" "$1" > $2', {inputValue: in2_1}, {inputValue: in2_2}, {outputPath: out2_1}]
         arguments:
-            in2_1: 21
+            in2_1: '21'
             in2_2: {taskOutput: {taskId: task 1, outputName: out1_1}}
       task 3:
         componentRef:
@@ -322,14 +322,14 @@ def test_load_nested_graph_components(self):
       graph out 1: {taskOutput: {taskId: task 3, outputName: out3_1}}
       graph out 2: {taskOutput: {taskId: task 1, outputName: out1_2}}
       graph out 3: {graphInput: graph in 2}
-      graph out 4: 42
+      graph out 4: '42'
 '''
         op = comp.load_component_from_text(component_text)
         task = op('graph 1', 'graph 2')
         self.assertIn('out3_1', str(task.outputs['graph out 1'])) # Checks that the outputs coming from tasks in nested subgraphs are properly resolved.
         self.assertIn('out1_2', str(task.outputs['graph out 2']))
         self.assertEqual(task.outputs['graph out 3'], 'graph 2')
-        self.assertEqual(task.outputs['graph out 4'], 42)
+        self.assertEqual(task.outputs['graph out 4'], '42')
 
 #TODO: Test task name conversion to Argo-compatible names
 

sdk/python/tests/components/test_python_op.py

@@ -135,7 +135,9 @@ def helper_test_component_against_func_using_local_call(self, func: Callable, op
 
         self.helper_test_component_using_local_call(op, arguments, expected_output_values_dict)
 
-    def helper_test_component_using_local_call(self, component_task_factory: Callable, arguments: dict, expected_output_values: dict):
+    def helper_test_component_using_local_call(self, component_task_factory: Callable, arguments: dict = None, expected_output_values: dict = None):
+        arguments = arguments or {}
+        expected_output_values = expected_output_values or {}
         with tempfile.TemporaryDirectory() as temp_dir_name:
             # Creating task from the component.
             # We do it in a special context that allows us to control the output file locations.
@@ -439,33 +441,29 @@ def add_multiply_two_numbers(a: float = 3, b: float = 5) -> NamedTuple('DummyNam
         self.assertEqual(component_spec.inputs[1].default, '5')
 
     def test_handling_default_value_of_none(self):
-        def assert_is_none(a, b, arg=None) -> int:
+        def assert_is_none(arg=None):
             assert arg is None
-            return 1
 
         func = assert_is_none
         op = comp.func_to_container_op(func)
-        self.helper_test_2_in_1_out_component_using_local_call(func, op)
+        self.helper_test_component_using_local_call(op)
 
 
-    def test_handling_complex_default_values_of_none(self):
+    def test_handling_complex_default_values(self):
         def assert_values_are_default(
-            a, b,
             singleton_param=None,
             function_param=ascii,
-            dict_param={'b': [2, 3, 4]},
+            dict_param: dict = {'b': [2, 3, 4]},
             func_call_param='_'.join(['a', 'b', 'c']),
-        ) -> int:
+        ):
             assert singleton_param is None
             assert function_param is ascii
             assert dict_param == {'b': [2, 3, 4]}
             assert func_call_param == '_'.join(['a', 'b', 'c'])
 
-            return 1
-
         func = assert_values_are_default
         op = comp.func_to_container_op(func)
-        self.helper_test_2_in_1_out_component_using_local_call(func, op)
+        self.helper_test_component_using_local_call(op)
 
 
     def test_handling_boolean_arguments(self):

sdk/python/tests/dsl/aws_extensions_tests.py

@@ -29,11 +29,11 @@ def test_default_aws_secret_name(self):
   def test_use_aws_secret(self):
       op1 = ContainerOp(name='op1', image='image')
       op1 = op1.apply(use_aws_secret('myaws-secret', 'key_id', 'access_key'))
-      assert len(op1.env_variables) == 2
+      assert len(op1.container.env) == 2
 
       index = 0
       for expected_name, expected_key in [('AWS_ACCESS_KEY_ID', 'key_id'), ('AWS_SECRET_ACCESS_KEY', 'access_key')]:
-          assert op1.env_variables[index].name == expected_name
-          assert op1.env_variables[index].value_from.secret_key_ref.name == 'myaws-secret'
-          assert op1.env_variables[index].value_from.secret_key_ref.key == expected_key
+          assert op1.container.env[index].name == expected_name
+          assert op1.container.env[index].value_from.secret_key_ref.name == 'myaws-secret'
+          assert op1.container.env[index].value_from.secret_key_ref.key == expected_key
           index += 1

sdk/python/tests/dsl/test_azure_extensions.py

@@ -26,13 +26,13 @@ def test_default_secret_name(self):
     def test_use_azure_secret(self):
         op1 = ContainerOp(name='op1', image='image')
         op1 = op1.apply(use_azure_secret('foo'))
-        assert len(op1.env_variables) == 4
+        assert len(op1.container.env) == 4
 
         index = 0
         for expected in ['AZ_SUBSCRIPTION_ID', 'AZ_TENANT_ID', 'AZ_CLIENT_ID', 'AZ_CLIENT_SECRET']:
-            assert op1.env_variables[index].name == expected
-            assert op1.env_variables[index].value_from.secret_key_ref.name == 'foo'
-            assert op1.env_variables[index].value_from.secret_key_ref.key == expected
+            assert op1.container.env[index].name == expected
+            assert op1.container.env[index].value_from.secret_key_ref.name == 'foo'
+            assert op1.container.env[index].value_from.secret_key_ref.key == expected
             index += 1
 
 if __name__ == '__main__':