From 643210d27410070da2cf0e3fdbf2325f8ff26404 Mon Sep 17 00:00:00 2001 From: Luke Granger-Brown Date: Wed, 9 May 2018 16:49:07 +0100 Subject: [PATCH] Add string output support for multifile output (#220) * Add test support for multi-file output. * Add -update support for multi-file output tests. * Add support for string output in multi-file output mode. * Rename 'stringOutput' to 'stringOutputMode' to better express what it does * Refactor main_test to make it less nested. This also causes the -update flag to output a list of files which have been updated. This does not include the paths which are deleted for multi-file tests. --- interpreter.go | 29 ++- main_test.go | 210 ++++++++++++++++---- testdata/multi.golden/bar.json | 7 + testdata/multi.golden/foo.json | 4 + testdata/multi.jsonnet | 13 ++ testdata/multi_string_output.golden/bar.txt | 1 + testdata/multi_string_output.golden/foo.txt | 1 + testdata/multi_string_output.jsonnet | 6 + vm.go | 6 +- 9 files changed, 225 insertions(+), 52 deletions(-) create mode 100644 testdata/multi.golden/bar.json create mode 100644 testdata/multi.golden/foo.json create mode 100644 testdata/multi.jsonnet create mode 100644 testdata/multi_string_output.golden/bar.txt create mode 100644 testdata/multi_string_output.golden/foo.txt create mode 100644 testdata/multi_string_output.jsonnet diff --git a/interpreter.go b/interpreter.go index 12a6ff951..c7fadd8c6 100644 --- a/interpreter.go +++ b/interpreter.go @@ -749,7 +749,7 @@ func (i *interpreter) manifestString(buf *bytes.Buffer, trace *TraceElement, v v } } -func (i *interpreter) manifestAndSerializeMulti(trace *TraceElement, v value) (r map[string]string, err error) { +func (i *interpreter) manifestAndSerializeMulti(trace *TraceElement, v value, stringOutputMode bool) (r map[string]string, err error) { r = make(map[string]string) json, err := i.manifestJSON(trace, v) if err != nil { @@ -758,10 +758,21 @@ func (i *interpreter) manifestAndSerializeMulti(trace *TraceElement, v value) (r switch json := json.(type) { case map[string]interface{}: for filename, fileJSON := range json { - var buf bytes.Buffer - serializeJSON(fileJSON, true, "", &buf) - buf.WriteString("\n") - r[filename] = buf.String() + if stringOutputMode { + switch val := fileJSON.(type) { + case string: + r[filename] = val + default: + msg := fmt.Sprintf("multi mode: top-level object's key %s has a value of type %T, "+ + "should be a string", filename, val) + return r, makeRuntimeError(msg, i.getCurrentStackTrace(trace)) + } + } else { + var buf bytes.Buffer + serializeJSON(fileJSON, true, "", &buf) + buf.WriteString("\n") + r[filename] = buf.String() + } } default: msg := fmt.Sprintf("multi mode: top-level object was a %s, "+ @@ -972,7 +983,7 @@ func evaluateAux(i *interpreter, node ast.Node, tla vmExtMap) (value, *TraceElem // TODO(sbarzowski) this function takes far too many arguments - build interpreter in vm instead func evaluate(node ast.Node, ext vmExtMap, tla vmExtMap, nativeFuncs map[string]*NativeFunction, - maxStack int, importer Importer, stringOutput bool) (string, error) { + maxStack int, importer Importer, stringOutputMode bool) (string, error) { i, err := buildInterpreter(ext, nativeFuncs, maxStack, importer) if err != nil { @@ -985,7 +996,7 @@ func evaluate(node ast.Node, ext vmExtMap, tla vmExtMap, nativeFuncs map[string] } var buf bytes.Buffer - if stringOutput { + if stringOutputMode { err = i.manifestString(&buf, manifestationTrace, result) } else { err = i.manifestAndSerializeJSON(&buf, manifestationTrace, result, true, "") @@ -999,7 +1010,7 @@ func evaluate(node ast.Node, ext vmExtMap, tla vmExtMap, nativeFuncs map[string] // TODO(sbarzowski) this function takes far too many arguments - build interpreter in vm instead func evaluateMulti(node ast.Node, ext vmExtMap, tla vmExtMap, nativeFuncs map[string]*NativeFunction, - maxStack int, importer Importer, stringOutput bool) (map[string]string, error) { + maxStack int, importer Importer, stringOutputMode bool) (map[string]string, error) { i, err := buildInterpreter(ext, nativeFuncs, maxStack, importer) if err != nil { @@ -1011,7 +1022,7 @@ func evaluateMulti(node ast.Node, ext vmExtMap, tla vmExtMap, nativeFuncs map[st return nil, err } - return i.manifestAndSerializeMulti(manifestationTrace, result) + return i.manifestAndSerializeMulti(manifestationTrace, result, stringOutputMode) } // TODO(sbarzowski) this function takes far too many arguments - build interpreter in vm instead diff --git a/main_test.go b/main_test.go index f9e2494c1..cf90fcefd 100644 --- a/main_test.go +++ b/main_test.go @@ -98,15 +98,20 @@ var nativeError = &NativeFunction{ } type jsonnetInput struct { - name string - input []byte - eKind evalKind - extVars map[string]string - extCode map[string]string + name string + input []byte + eKind evalKind + stringOutputMode bool + extVars map[string]string + extCode map[string]string } type jsonnetResult struct { - output string + // One of output or outputMulti is populated. + // If isError is set, the error is stored in output. + output string + outputMulti map[string]string + isError bool } @@ -114,6 +119,7 @@ func runInternalJsonnet(i jsonnetInput) jsonnetResult { vm := MakeVM() errFormatter := termErrorFormatter{pretty: true, maxStackTraceSize: 9} + vm.StringOutput = i.stringOutputMode for name, value := range i.extVars { vm.ExtVar(name, value) } @@ -124,29 +130,32 @@ func runInternalJsonnet(i jsonnetInput) jsonnetResult { vm.NativeFunction(jsonToString) vm.NativeFunction(nativeError) - var output string - rawOutput, err := vm.evaluateSnippet(i.name, string(i.input), i.eKind) - var isError bool - if err != nil { + switch { + case err != nil: // TODO(sbarzowski) perhaps somehow mark that we are processing // an error. But for now we can treat them the same. - output = errFormatter.Format(err) - output += "\n" - isError = true - } else { - output = rawOutput.(string) - isError = false - } - - return jsonnetResult{ - output: output, - isError: isError, + return jsonnetResult{ + output: errFormatter.Format(err) + "\n", + isError: true, + } + case i.eKind == evalKindMulti: + return jsonnetResult{ + outputMulti: rawOutput.(map[string]string), + } + default: + return jsonnetResult{ + output: rawOutput.(string), + } } } +// TODO(lukegb) CLI test support is presently completely broken: fix? func runJsonnetCommand(i jsonnetInput) jsonnetResult { // TODO(sbarzowski) Special handling of errors (which may differ between versions) + if i.eKind != evalKindRegular { + panic(fmt.Sprintf("eKind must be evalKindRegular for jsonnet CLI testing; was %v", i.eKind)) + } input := bytes.NewBuffer(i.input) var output bytes.Buffer isError := false @@ -181,8 +190,115 @@ func runJsonnet(i jsonnetInput) jsonnetResult { return runInternalJsonnet(i) } -func runTest(t *testing.T, test *mainTest) { +func compareGolden(result string, golden []byte) (string, bool) { + if bytes.Compare(golden, []byte(result)) != 0 { + // TODO(sbarzowski) better reporting of differences in whitespace + // missing newline issues can be very subtle now + return diff(result, string(golden)), true + } + return "", false +} + +func writeFile(path string, content []byte, mode os.FileMode) (changed bool, err error) { + old, err := ioutil.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + return false, err + } + if bytes.Compare(old, content) == 0 && !os.IsNotExist(err) { + return false, nil + } + if err := ioutil.WriteFile(path, content, mode); err != nil { + return false, err + } + return true, nil +} + +func compareSingleGolden(path string, result jsonnetResult) []error { + if result.outputMulti != nil { + return []error{fmt.Errorf("outputMulti is populated in a single-file test for %v", path)} + } + golden, err := ioutil.ReadFile(path) + if err != nil { + return []error{fmt.Errorf("reading file %s: %v", path, err)} + } + if diff, hasDiff := compareGolden(result.output, golden); hasDiff { + return []error{fmt.Errorf("golden file %v has diff:\n%v", path, diff)} + } + return nil +} + +func updateSingleGolden(path string, result jsonnetResult) (updated []string, err error) { + if result.outputMulti != nil { + return nil, fmt.Errorf("outputMulti is populated in a single-file test for %v", path) + } + changed, err := writeFile(path, []byte(result.output), 0666) + if err != nil { + return nil, fmt.Errorf("updating golden file %v: %v", path, err) + } + if changed { + return []string{path}, nil + } + return nil, nil +} +func compareMultifileGolden(path string, result jsonnetResult) []error { + expectFiles, err := ioutil.ReadDir(path) + if err != nil { + return []error{fmt.Errorf("reading golden dir %v: %v", path, err)} + } + goldenContent := map[string][]byte{} + var errs []error + for _, f := range expectFiles { + golden, err := ioutil.ReadFile(filepath.Join(path, f.Name())) + if err != nil { + return []error{fmt.Errorf("reading file %s: %v", f.Name(), err)} + } + if _, ok := result.outputMulti[f.Name()]; !ok { + errs = append(errs, fmt.Errorf("jsonnet did not output expected file %v", f.Name())) + continue + } + goldenContent[f.Name()] = golden + } + for fn, content := range result.outputMulti { + if _, ok := goldenContent[fn]; !ok { + errs = append(errs, fmt.Errorf("jsonnet outputted file %v which does not exist in goldens", fn)) + continue + } + if diff, hasDiff := compareGolden(content, goldenContent[fn]); hasDiff { + errs = append(errs, fmt.Errorf("golden file %v has diff:\n%v", fn, diff)) + } + } + return errs +} + +func updateMultifileGolden(path string, result jsonnetResult) ([]string, error) { + expectFiles, err := ioutil.ReadDir(path) + if err != nil { + return nil, fmt.Errorf("reading golden directory %v: %v", path, err) + } + var updatedFiles []string + for fn, content := range result.outputMulti { + updated, err := writeFile(filepath.Join(path, fn), []byte(content), 0666) + if err != nil { + return nil, fmt.Errorf("updating golden file %v: %v", fn, err) + } + if updated { + updatedFiles = append(updatedFiles, filepath.Join(path, fn)) + } + } + // Delete excess files + for _, f := range expectFiles { + if _, ok := result.outputMulti[f.Name()]; ok { + continue + } + if err := os.Remove(filepath.Join(path, f.Name())); err != nil { + return nil, fmt.Errorf("removing golden file %v: %v", f.Name(), err) + } + } + return updatedFiles, nil +} + +func runTest(t *testing.T, test *mainTest) { read := func(file string) []byte { bytz, err := ioutil.ReadFile(file) if err != nil { @@ -193,32 +309,46 @@ func runTest(t *testing.T, test *mainTest) { input := read(test.input) + eKind := evalKindRegular + compareFunc := compareSingleGolden + updateFunc := updateSingleGolden + + // If the golden path is a directory, this is a multi-test. + if info, err := os.Stat(test.golden); err == nil && info.IsDir() { + eKind = evalKindMulti + compareFunc = compareMultifileGolden + updateFunc = updateMultifileGolden + } + result := runJsonnet(jsonnetInput{ - name: test.name, - input: input, - eKind: evalKindRegular, - extVars: test.meta.extVars, - extCode: test.meta.extCode, + name: test.name, + input: input, + eKind: eKind, + stringOutputMode: strings.HasSuffix(test.golden, "_string_output.golden"), + extVars: test.meta.extVars, + extCode: test.meta.extCode, }) - // TODO(sbarzowski) report which files were updated + if eKind == evalKindMulti && result.isError { + // If it's an error, then result.output is populated instead. + // Since we use the golden file being a directory to determine if we + // should run in multi-file mode, we put the output into an "error" file instead. + result.outputMulti = map[string]string{"error": result.output} + result.output = "" + } + if *update { - err := ioutil.WriteFile(test.golden, []byte(result.output), 0666) + updated, err := updateFunc(test.golden, result) if err != nil { - t.Errorf("error updating golden files: %v", err) + t.Error(err) + } + for _, updatedFile := range updated { + fmt.Fprintf(os.Stderr, "updated golden %v\n", updatedFile) } return } - golden := read(test.golden) - if bytes.Compare(golden, []byte(result.output)) != 0 { - // TODO(sbarzowski) better reporting of differences in whitespace - // missing newline issues can be very subtle now - t.Fail() - t.Errorf("Mismatch when running %s.jsonnet. Golden: %s\n", test.name, test.golden) - data := diff(result.output, string(golden)) - t.Errorf("diff %s jsonnet %s.jsonnet\n", test.golden, test.name) - t.Errorf(string(data)) - + for _, err := range compareFunc(test.golden, result) { + t.Error(err) } } diff --git a/testdata/multi.golden/bar.json b/testdata/multi.golden/bar.json new file mode 100644 index 000000000..f9e4099cf --- /dev/null +++ b/testdata/multi.golden/bar.json @@ -0,0 +1,7 @@ +{ + "foo": { + "bar": { + "baz": "yes" + } + } +} diff --git a/testdata/multi.golden/foo.json b/testdata/multi.golden/foo.json new file mode 100644 index 000000000..7488d76ec --- /dev/null +++ b/testdata/multi.golden/foo.json @@ -0,0 +1,4 @@ +{ + "baq": "27", + "baz": 3 +} diff --git a/testdata/multi.jsonnet b/testdata/multi.jsonnet new file mode 100644 index 000000000..5b358d08d --- /dev/null +++ b/testdata/multi.jsonnet @@ -0,0 +1,13 @@ +{ + "foo.json": { + baz: 3, + baq: "27", + }, + "bar.json": { + foo: { + bar: { + baz: "yes", + }, + }, + }, +} diff --git a/testdata/multi_string_output.golden/bar.txt b/testdata/multi_string_output.golden/bar.txt new file mode 100644 index 000000000..ba0e162e1 --- /dev/null +++ b/testdata/multi_string_output.golden/bar.txt @@ -0,0 +1 @@ +bar \ No newline at end of file diff --git a/testdata/multi_string_output.golden/foo.txt b/testdata/multi_string_output.golden/foo.txt new file mode 100644 index 000000000..cef102d5a --- /dev/null +++ b/testdata/multi_string_output.golden/foo.txt @@ -0,0 +1 @@ +foo. diff --git a/testdata/multi_string_output.jsonnet b/testdata/multi_string_output.jsonnet new file mode 100644 index 000000000..71c85b82c --- /dev/null +++ b/testdata/multi_string_output.jsonnet @@ -0,0 +1,6 @@ +{ + "foo.txt": ||| + foo. + |||, + "bar.txt": "bar", +} diff --git a/vm.go b/vm.go index 4a254fb3e..e0fd2a406 100644 --- a/vm.go +++ b/vm.go @@ -91,9 +91,9 @@ func (vm *VM) Importer(i Importer) { type evalKind int const ( - evalKindRegular = iota - evalKindMulti = iota - evalKindStream = iota + evalKindRegular evalKind = iota + evalKindMulti = iota + evalKindStream = iota ) func (vm *VM) evaluateSnippet(filename string, snippet string, kind evalKind) (output interface{}, err error) {