From cfdc996c16d23e968e9bb1ebcad4c81a17763fa8 Mon Sep 17 00:00:00 2001 From: Christian Kadner Date: Mon, 6 Jul 2020 09:14:53 -0700 Subject: [PATCH] Make unit tests run with Python 3.5 (#210) --- .travis.yml | 8 +++++ sdk/python/kfp_tekton/compiler/compiler.py | 6 ++-- sdk/python/tests/compiler/compiler_tests.py | 31 +++++++++++++++++-- sdk/python/tests/compiler/k8s_helper_tests.py | 12 +++---- .../compiler/testdata/big_data_passing.yaml | 22 ++++++------- .../compiler/testdata/withitem_nested.yaml | 5 ++- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/.travis.yml b/.travis.yml index c9b1d588250..8f8f9173ef3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,14 @@ matrix: include: + - name: "Unit tests, Python 3.5" + language: python + python: "3.5" + env: TOXENV=py35 + install: + - python3 -m pip install -e sdk/python + script: + - make unit_test - name: "Unit tests, Python 3.6" language: python python: "3.6" diff --git a/sdk/python/kfp_tekton/compiler/compiler.py b/sdk/python/kfp_tekton/compiler/compiler.py index 27560123178..fb49e358b87 100644 --- a/sdk/python/kfp_tekton/compiler/compiler.py +++ b/sdk/python/kfp_tekton/compiler/compiler.py @@ -318,7 +318,7 @@ def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeli for arg in args: param = {'name': arg.name} if arg.value is not None: - if isinstance(arg.value, (list, tuple)): + if isinstance(arg.value, (list, tuple, dict)): param['default'] = json.dumps(arg.value, sort_keys=True) else: param['default'] = str(arg.value) @@ -478,8 +478,8 @@ def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeli 'name': sanitize_k8s_name(pipeline.name or 'Pipeline', suffix_space=4), 'labels': get_default_telemetry_labels(), 'annotations': { - 'tekton.dev/output_artifacts': json.dumps(self.output_artifacts), - 'tekton.dev/input_artifacts': json.dumps(self.input_artifacts), + 'tekton.dev/output_artifacts': json.dumps(self.output_artifacts, sort_keys=True), + 'tekton.dev/input_artifacts': json.dumps(self.input_artifacts, sort_keys=True), 'sidecar.istio.io/inject': 'false' # disable Istio inject since Tekton cannot run with Istio sidecar } }, diff --git a/sdk/python/tests/compiler/compiler_tests.py b/sdk/python/tests/compiler/compiler_tests.py index e4ad06c42b8..1f419db27ac 100644 --- a/sdk/python/tests/compiler/compiler_tests.py +++ b/sdk/python/tests/compiler/compiler_tests.py @@ -12,14 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import logging import os import re import shutil +import sys import tempfile import textwrap import unittest import yaml + from os import environ as env from kfp_tekton import compiler @@ -230,8 +233,14 @@ def test_katib_workflow(self): """ Test compiling a katib workflow. """ - from .testdata.katib import mnist_hpo - self._test_pipeline_workflow(mnist_hpo, 'katib.yaml') + # dictionaries and lists do not preserve insertion order before Python 3.6. + # katib.py uses (string-serialized) dictionaries containing dsl.PipelineParam objects + # which can't be JSON-serialized so using json.dumps(sorted) is not an alternative + if sys.version_info < (3, 6, 0): + logging.warning("Skipping katib workflow test for Python version < 3.6.0") + else: + from .testdata.katib import mnist_hpo + self._test_pipeline_workflow(mnist_hpo, 'katib.yaml') def test_load_from_yaml_workflow(self): """ @@ -352,4 +361,20 @@ def _verify_compiled_workflow(self, golden_yaml_file, compiled_workflow): with open(golden_yaml_file, 'r') as f: golden = yaml.safe_load(f) self.maxDiff = None - self.assertEqual(golden, compiled_workflow) + + # sort dicts and lists, insertion order was not guaranteed before Python 3.6 + if sys.version_info < (3, 6, 0): + def sort_items(obj): + from collections import OrderedDict + if isinstance(obj, dict): + return OrderedDict({k: sort_items(v) for k, v in sorted(obj.items())}) + elif isinstance(obj, list): + return sorted([sort_items(o) for o in obj], key=lambda x: str(x)) + else: + return obj + golden = sort_items(golden) + compiled_workflow = sort_items(compiled_workflow) + + self.assertEqual(golden, compiled_workflow, + msg="\n===[ " + golden_yaml_file.split(os.path.sep)[-1] + " ]===\n" + + json.dumps(compiled_workflow, indent=2)) diff --git a/sdk/python/tests/compiler/k8s_helper_tests.py b/sdk/python/tests/compiler/k8s_helper_tests.py index db9498490da..db3cdcb7c73 100644 --- a/sdk/python/tests/compiler/k8s_helper_tests.py +++ b/sdk/python/tests/compiler/k8s_helper_tests.py @@ -66,13 +66,13 @@ def test_sanitize_k8s_labels(self): "My-other-hobbies": "eating-drinking.-sleeping" } self.assertEqual( - list(map(lambda k: sanitize_k8s_name(k, allow_capital_underscore=True, allow_dot=True, - allow_slash=True, max_length=253), labels.keys())), - list(expected_labels.keys())) + sorted(map(lambda k: sanitize_k8s_name(k, allow_capital_underscore=True, allow_dot=True, + allow_slash=True, max_length=253), labels.keys())), + sorted(expected_labels.keys())) self.assertEqual( - list(map(lambda v: sanitize_k8s_name(v, allow_capital_underscore=True, allow_dot=True, - allow_slash=False, max_length=63), labels.values())), - list(expected_labels.values())) + sorted(map(lambda v: sanitize_k8s_name(v, allow_capital_underscore=True, allow_dot=True, + allow_slash=False, max_length=63), labels.values())), + sorted(expected_labels.values())) def test_sanitize_k8s_annotations(self): annotation_keys = { diff --git a/sdk/python/tests/compiler/testdata/big_data_passing.yaml b/sdk/python/tests/compiler/testdata/big_data_passing.yaml index 6d6801798b4..4007b177ff9 100644 --- a/sdk/python/tests/compiler/testdata/big_data_passing.yaml +++ b/sdk/python/tests/compiler/testdata/big_data_passing.yaml @@ -18,21 +18,21 @@ metadata: annotations: pipelines.kubeflow.org/pipeline_spec: '{"name": "File passing pipelines"}' sidecar.istio.io/inject: 'false' - tekton.dev/input_artifacts: '{"print-text": [{"name": "repeat-line-output_text", - "parent_task": "repeat-line"}], "print-text-2": [{"name": "split-text-lines-odd_lines", - "parent_task": "split-text-lines"}], "print-text-3": [{"name": "split-text-lines-even_lines", + tekton.dev/input_artifacts: '{"print-params": [{"name": "gen-params-output", "parent_task": + "gen-params"}], "print-text": [{"name": "repeat-line-output_text", "parent_task": + "repeat-line"}], "print-text-2": [{"name": "split-text-lines-odd_lines", "parent_task": + "split-text-lines"}], "print-text-3": [{"name": "split-text-lines-even_lines", "parent_task": "split-text-lines"}], "print-text-4": [{"name": "write-numbers-numbers", - "parent_task": "write-numbers"}], "sum-numbers": [{"name": "write-numbers-numbers", "parent_task": "write-numbers"}], "print-text-5": [{"name": "sum-numbers-output", - "parent_task": "sum-numbers"}], "print-params": [{"name": "gen-params-output", - "parent_task": "gen-params"}]}' - tekton.dev/output_artifacts: '{"repeat-line": [{"name": "repeat-line-output_text", + "parent_task": "sum-numbers"}], "sum-numbers": [{"name": "write-numbers-numbers", + "parent_task": "write-numbers"}]}' + tekton.dev/output_artifacts: '{"gen-params": [{"name": "gen-params-output", "path": + "/tmp/outputs/Output/data"}], "repeat-line": [{"name": "repeat-line-output_text", "path": "/tmp/outputs/output_text/data"}], "split-text-lines": [{"name": "split-text-lines-even_lines", "path": "/tmp/outputs/even_lines/data"}, {"name": "split-text-lines-odd_lines", - "path": "/tmp/outputs/odd_lines/data"}], "write-numbers": [{"name": "write-numbers-numbers", - "path": "/tmp/outputs/numbers/data"}], "sum-numbers": [{"name": "sum-numbers-output", - "path": "/tmp/outputs/Output/data"}], "gen-params": [{"name": "gen-params-output", - "path": "/tmp/outputs/Output/data"}]}' + "path": "/tmp/outputs/odd_lines/data"}], "sum-numbers": [{"name": "sum-numbers-output", + "path": "/tmp/outputs/Output/data"}], "write-numbers": [{"name": "write-numbers-numbers", + "path": "/tmp/outputs/numbers/data"}]}' labels: pipelines.kubeflow.org/pipeline-sdk-type: kfp name: file-passing-pipelines diff --git a/sdk/python/tests/compiler/testdata/withitem_nested.yaml b/sdk/python/tests/compiler/testdata/withitem_nested.yaml index 87c83fbe142..3e566b730a7 100644 --- a/sdk/python/tests/compiler/testdata/withitem_nested.yaml +++ b/sdk/python/tests/compiler/testdata/withitem_nested.yaml @@ -20,10 +20,9 @@ metadata: "optional": true, "type": "Integer"}], "name": "my-pipeline"}' sidecar.istio.io/inject: 'false' tekton.dev/input_artifacts: '{"my-in-coop1": [{"name": "loop-item-param-00000001-subvar-a", + "parent_task": null}], "my-in-coop2": [{"name": "loop-item-param-00000001-subvar-b", "parent_task": null}], "my-inner-inner-coop": [{"name": "loop-item-param-00000001-subvar-a", - "parent_task": null}, {"name": "loop-item-param-00000002", "parent_task": null}], - "my-in-coop2": [{"name": "loop-item-param-00000001-subvar-b", "parent_task": - null}]}' + "parent_task": null}, {"name": "loop-item-param-00000002", "parent_task": null}]}' tekton.dev/output_artifacts: '{}' labels: pipelines.kubeflow.org/pipeline-sdk-type: kfp