Update jsonnet tests so that it tests all files under kubeflow #480
Conversation
* This will catch any syntax errors in *sonnet files. * Skip all ksonnet prototypes since they need additional deps /cc @lluunn
|
/retest |
testing/test_jsonnet.py
Outdated
| # except ksonnet prototype definitions - they require additional | ||
| # dependencies | ||
| def should_test(f): | ||
| parts = f.split('.') |
| @@ -38,18 +38,32 @@ | |||
| from kubeflow.testing import test_util | |||
| from kubeflow.testing import util | |||
|
|
|||
| # We should test all files which end in .jsonnet or .libsonnet | |||
| # except ksonnet prototype definitions - they require additional | |||
There was a problem hiding this comment.
Aren't the files in prototypes jsonnet, and otherwise libsonnet?
There was a problem hiding this comment.
We cannot test the prototypes directly since they are not valid jsonnet. For example: ksonnet injects the params and the env variable into the prototype when a component is generated from that prototype.
The idea is that we want to test everything(jsonnet and libsonnet file) under the kubeflow/ directory unless it is a ksonnet prototype.
There was a problem hiding this comment.
I mean, is the following true:
.jsonnet --> ksonnet prototype, cannot test
.libsonnet --> valid, can test
I manually checked, seems to be the case?
There was a problem hiding this comment.
Yes. We shouldn't test any .jsonnet under .../prototypes/ since they are ksonnet prototypes.
testing/test_jsonnet.py
Outdated
| # except ksonnet prototype definitions - they require additional | ||
| # dependencies | ||
| def should_test(f): | ||
| parts = os.path.splitext(f) |
There was a problem hiding this comment.
_, ext = os.path.splitext(f)
Then use ext as parts[1] below.
I mean, shouldn't we test only libsonnet? So should_test will be return ext == '.libsonnet'
There was a problem hiding this comment.
We also need to run some eval the .jsonnet files in the tests directory : https://github.com/kubeflow/kubeflow/tree/master/kubeflow/core/tests
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lluunn The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Init commit * Add Katib Client * Add GetConfigMap func Move templates const * Change folder for Katib client * Delete old client * Change name for default templates
/cc @lluunn
This change is