Add tests for jsonnet functions and fix parseYAML. #195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- funcs.go was missing tests for the jsonnet native functions, this PR adds some basic tests exercising some generic classses of arguments - `parseYAML` seems to have been doing one extra step of marshalling the YAML documents into JSON, which it probably shouldn't do and this PR removes the functionality. W.r.t. second point, the current implementation looks like manifestJsonFrom Yaml or similar -- is there any use case for that that would be worth adding it? Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
|
This should fix #194 . |
sbarzowski
reviewed
Feb 6, 2020
pkg/jsonnet/native/funcs.go
Outdated
| return nil, err | ||
| } | ||
| ret = append(ret, jsonDoc) | ||
| ret = append(ret, doc) |
There was a problem hiding this comment.
Wouldn't this result in problems further down the line? Jsonnet expects the result of a native function to comply with the standard Go representation, that is:
bool, for JSON booleans
float64, for JSON numbers
string, for JSON strings
[]interface{}, for JSON arrays
map[string]interface{}, for JSON objects
nil for JSON null
Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
This normalizes YAML data to a form compatible with JSON -- that is, `int` gets turned into `float64` etc. Note: some cases this code still fails with: ``` evaluating jsonnet: unexpected end of JSON input ``` Interestingly enough, the same file (CRDs from cert-manager) fails when used from a jsonnet library for cert-manager, but succeeds when loaded from a dummy libsonnet file. Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
The `err` variable returned by `jsonnet.EvaluateFile` call was silently ignored, making tanka fail with mysterious ``` evaluating jsonnet: unexpected end of JSON input ``` This commit makes the code properly handle errors. Thanks to @sh0rez for a quick pointer. Signed-off-by: Milan Plzik <milan.plzik@grafana.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
funcs.go was missing tests for the jsonnet native functions, this PR
adds some basic tests exercising some generic classses of arguments
parseYAMLseems to have been doing one extra step of marshalling theYAML documents into JSON, which it probably shouldn't do and this PR
removes the functionality.
W.r.t. second point, the current implementation looks like
manifestJsonFrom Yaml or similar -- is there any use case for that that
would be worth adding it?