Scuemata: Checking json validity by enabling skipped tests (#34385)

* Make sure we don't skip any tests - refactoring

* Remove commented lines

* Move test folder

pkg/schema/load/dashboard.go

@@ -5,18 +5,10 @@ import (
 	"fmt"
 
 	"cuelang.org/go/cue"
-	errs "cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/load"
 	"github.com/grafana/grafana/pkg/schema"
 )
 
-// cueError wraps errors caused by malformed cue files.
-type cueError struct {
-	errors   []errs.Error
-	filename string
-	line     int
-}
-
 var panelSubpath = cue.MakePath(cue.Def("#Panel"))
 
 func defaultOverlay(p BaseLoadPaths) (map[string]load.Source, error) {
@@ -47,9 +39,9 @@ func BaseDashboardFamily(p BaseLoadPaths) (schema.VersionedCueSchema, error) {
 	cfg := &load.Config{Overlay: overlay}
 	inst, err := rt.Build(load.Instances([]string{"/cue/data/gen.cue"}, cfg)[0])
 	if err != nil {
-		cueErrors := wrapCUEError(err)
+		cueError := schema.WrapCUEError(err)
 		if err != nil {
-			return nil, fmt.Errorf("errors: %q, in file: %s, on line: %d", cueErrors.errors, cueErrors.filename, cueErrors.line)
+			return nil, cueError
 		}
 	}
 
@@ -196,15 +188,3 @@ type CompositeDashboardSchema interface {
 	schema.VersionedCueSchema
 	LatestPanelSchemaFor(id string) (schema.VersionedCueSchema, error)
 }
-
-func wrapCUEError(err error) cueError {
-	var cErr errs.Error
-	if ok := errors.As(err, &cErr); ok {
-		return cueError{
-			errors:   errs.Errors(err),
-			filename: errs.Errors(err)[0].Position().File().Name(),
-			line:     errs.Errors(err)[0].Position().Line(),
-		}
-	}
-	return cueError{}
-}

pkg/schema/load/load_test.go

@@ -6,16 +6,14 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"testing/fstest"
 
-	"github.com/grafana/grafana"
 	"github.com/grafana/grafana/pkg/schema"
+	"github.com/laher/mergefs"
 	"github.com/stretchr/testify/require"
 )
 
-var p BaseLoadPaths = BaseLoadPaths{
-	BaseCueFS:       grafana.CoreSchema,
-	DistPluginCueFS: grafana.PluginSchema,
-}
+var p = GetDefaultLoadPaths()
 
 // Basic well-formedness tests on core scuemata.
 func TestScuemataBasics(t *testing.T) {
@@ -48,9 +46,6 @@ func TestScuemataBasics(t *testing.T) {
 }
 
 func TestDashboardValidity(t *testing.T) {
-	// TODO FIXME remove this once we actually have dashboard schema filled in
-	// enough that the tests pass, lol
-	t.Skip()
 	validdir := os.DirFS(filepath.Join("testdata", "artifacts", "dashboards"))
 
 	dash, err := BaseDashboardFamily(p)
@@ -87,9 +82,6 @@ func TestDashboardValidity(t *testing.T) {
 func TestPanelValidity(t *testing.T) {
 	validdir := os.DirFS(filepath.Join("testdata", "artifacts", "panels"))
 
-	// dash, err := BaseDashboardFamily(p)
-	// require.NoError(t, err, "error while loading base dashboard scuemata")
-
 	ddash, err := DistDashboardFamily(p)
 	require.NoError(t, err, "error while loading dist dashboard scuemata")
 
@@ -111,7 +103,6 @@ func TestPanelValidity(t *testing.T) {
 		t.Run(path, func(t *testing.T) {
 			// TODO FIXME stop skipping once we actually have the schema filled in
 			// enough that the tests pass, lol
-			t.Skip()
 
 			b, err := validdir.Open(path)
 			require.NoError(t, err, "failed to open panel file")
@@ -125,22 +116,26 @@ func TestPanelValidity(t *testing.T) {
 }
 
 func TestCueErrorWrapper(t *testing.T) {
-	t.Run("Testing scuemata validity with valid cue schemas", func(t *testing.T) {
-		tempDir := os.DirFS(filepath.Join("testdata", "malformed_cue"))
+	t.Run("Testing cue error wrapper", func(t *testing.T) {
+		a := fstest.MapFS{
+			"cue/data/gen.cue": &fstest.MapFile{Data: []byte("{;;;;;;;;}")},
+		}
+
+		filesystem := mergefs.Merge(a, GetDefaultLoadPaths().BaseCueFS)
 
 		var baseLoadPaths = BaseLoadPaths{
-			BaseCueFS:       tempDir,
+			BaseCueFS:       filesystem,
 			DistPluginCueFS: GetDefaultLoadPaths().DistPluginCueFS,
 		}
 
 		_, err := BaseDashboardFamily(baseLoadPaths)
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "in file")
-		require.Contains(t, err.Error(), "on line")
+		require.Contains(t, err.Error(), "line: ")
 
 		_, err = DistDashboardFamily(baseLoadPaths)
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "in file")
-		require.Contains(t, err.Error(), "on line")
+		require.Contains(t, err.Error(), "line: ")
 	})
 }

pkg/schema/load/testdata/artifacts/dashboards/basic.json

@@ -58,7 +58,6 @@
             "filterable": false
           },
           "decimals": 3,
-          "mappings": [],
           "unit": "watt"
         },
         "overrides": [
@@ -150,4 +149,4 @@
   "title": "with table",
   "uid": "emal8gQMz",
   "version": 2
-}
+}

pkg/schema/load/testdata/artifacts/panels/basic-table.json

@@ -7,7 +7,6 @@
         "filterable": false
       },
       "decimals": 3,
-      "mappings": [],
       "unit": "watt"
     },
     "overrides": [
@@ -82,4 +81,4 @@
     "maj": 0,
     "min": 0
   }
-}
+}

pkg/schema/load/testdata/malformed_cue/cue/data/gen.cue

@@ -1 +0,0 @@
-;;;;;;;

pkg/schema/load/testdata/malformed_cue/cue/scuemata/panel-plugin.cue

@@ -1,22 +0,0 @@
-package scuemata
-
-// Definition of the shape of a panel plugin's schema declarations in its
-// schema.cue file.
-//
-// Note that these keys do not appear directly in any real JSON artifact;
-// rather, they are composed into panel structures as they are defined within
-// the larger Dashboard schema.
-#PanelSchema: {
-	PanelOptions: {...}
-	PanelFieldConfig?: {...}
-	...
-}
-
-// A lineage of panel schema
-#PanelLineage: [#PanelSchema, ...#PanelSchema]
-
-// Panel plugin-specific Family
-#PanelFamily: {
-	lineages: [#PanelLineage, ...#PanelLineage]
-	migrations: [...#Migration]
-}

pkg/schema/load/testdata/malformed_cue/cue/scuemata/scuemata.cue

@@ -1,60 +0,0 @@
-package scuemata
-
-// A family is a collection of schemas that specify a single kind of object,
-// allowing evolution of the canonical schema for that kind of object over time.
-//
-// The schemas are organized into a list of Lineages, which are themselves ordered
-// lists of schemas where each schema with its predecessor in the lineage.
-//
-// If it is desired to define a schema with a breaking schema relative to its
-// predecessors, a new Lineage must be created, as well as a Migration that defines
-// a mapping to the new schema from the latest schema in prior Lineage.
-//
-// The version number of a schema is not controlled by the schema itself, but by
-// its position in the list of lineages - e.g., 0.0 corresponds to the first
-// schema in the first lineage.
-#Family: {
-	lineages: [#Lineage, ...#Lineage]
-	migrations: [...#Migration]
-	let lseq = lineages[len(lineages)-1]
-	latest: #LastSchema & {_p: lseq}
-}
-
-// A Lineage is a non-empty list containing an ordered series of schemas that
-// all describe a single kind of object, where each schema is backwards
-// compatible with its predecessor.
-#Lineage: [{...}, ...{...}]
-
-#LastSchema: {
-	_p: #Lineage
-	_p[len(_p)-1]
-}
-
-// A Migration defines a relation between two schemas, "_from" and "_to". The
-// relation expresses any complex mappings that must be performed to
-// transform an input artifact valid with respect to the _from schema, into
-// an artifact valid with respect to the _to schema. This is accomplished
-// in two stages:
-//   1. A Migration is initially defined by passing in schemas for _from and _to,
-//      and mappings that translate _from to _to are defined in _rel. 
-//   2. A concrete object may then be unified with _to, resulting in its values
-//      being mapped onto "result" by way of _rel.
-//
-// This is the absolute simplest possible definition of a Migration. It's
-// incumbent on the implementor to manually ensure the correctness and
-// completeness of the mapping. The primary value in defining such a generic
-// structure is to allow comparably generic logic for migrating concrete
-// artifacts through schema changes.
-//
-// If _to isn't backwards compatible (accretion-only) with _from, then _rel must
-// explicitly enumerate every field in _from and map it to a field in _to, even
-// if they're identical. This is laborious for anything outside trivially tiny
-// schema. We'll want to eventually add helpers for whitelisting or blacklisting
-// of paths in _from, so that migrations of larger schema can focus narrowly on
-// the points of actual change.
-#Migration: {
-	from: {...}
-	to: {...}
-	rel: {...}
-	result: to & rel
-}

pkg/schema/schema.go

@@ -8,11 +8,28 @@ import (
 	"strings"
 
 	"cuelang.org/go/cue"
+	errs "cuelang.org/go/cue/errors"
 	cuejson "cuelang.org/go/pkg/encoding/json"
 )
 
 var rt = &cue.Runtime{}
 
+// CueError wraps Errors caused by malformed cue files.
+type CueError struct {
+	ErrorMap map[int]string
+}
+
+// Error func needed to implement standard golang error
+func (cErr *CueError) Error() string {
+	var errorString string
+	if cErr.ErrorMap != nil {
+		for k, v := range cErr.ErrorMap {
+			errorString = errorString + fmt.Sprintf("line: %d, %s \n", k, v)
+		}
+	}
+	return errorString
+}
+
 // CueSchema represents a single, complete CUE-based schema that can perform
 // operations on Resources.
 //
@@ -93,6 +110,10 @@ func SearchAndValidate(s VersionedCueSchema, v interface{}) (VersionedCueSchema,
 	// collates all the individual errors, relates them to the schema that
 	// produced them, and ideally deduplicates repeated errors across each
 	// schema.
+	cueErrors := WrapCUEError(err)
+	if err != nil {
+		return nil, cueErrors
+	}
 	return nil, err
 }
 
@@ -402,4 +423,24 @@ type Resource struct {
 	Value interface{}
 }
 
+// WrapCUEError is a wrapper for cueErrors that occur and are not self explanatory.
+// If an error is of type cueErr, then iterate through the error array, export line number
+// and filename, otherwise return usual error.
+func WrapCUEError(err error) error {
+	var cErr errs.Error
+	m := make(map[int]string)
+	if ok := errors.As(err, &cErr); ok {
+		for _, e := range errs.Errors(err) {
+			if e.Position().File() != nil {
+				line := e.Position().Line()
+				m[line] = fmt.Sprintf("%q: in file %s", err, e.Position().File().Name())
+			}
+		}
+	}
+	if len(m) != 0 {
+		return &CueError{m}
+	}
+	return err
+}
+
 // TODO add migrator with SearchOption for stopping criteria