Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Standardise schema migration CLI errors #1682

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ ifdef BUILD_TAGS
BUILD_FLAGS+=-tags $(BUILD_TAGS)
endif

TEST_FLAGS=-race -shuffle=on -timeout 70s
TEST_FLAGS=-race -shuffle=on -timeout 120s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: what caused this bump?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More tests == even slower :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess a bunch of CLI tests


PLAYGROUND_DIRECTORY=playground
LENS_TEST_DIRECTORY=tests/integration/schema/migrations
Expand Down Expand Up @@ -243,6 +243,7 @@ test\:lens:

.PHONY: test\:cli
test\:cli:
@$(MAKE) deps:lens
gotestsum --format testname -- ./$(CLI_TEST_DIRECTORY)/... $(TEST_FLAGS)

# Using go-acc to ensure integration tests are included.
Expand Down
14 changes: 11 additions & 3 deletions cli/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

package cli

import "github.com/sourcenetwork/defradb/errors"
import (
"strings"

"github.com/sourcenetwork/defradb/errors"
)

const (
errMissingArg string = "missing argument"
Expand Down Expand Up @@ -61,8 +65,12 @@ func NewErrMissingArg(name string) error {
return errors.New(errMissingArg, errors.NewKV("Name", name))
}

func NewErrMissingArgs(count int, provided int) error {
return errors.New(errMissingArgs, errors.NewKV("Required", count), errors.NewKV("Provided", provided))
func NewErrMissingArgs(names []string) error {
return errors.New(errMissingArgs, errors.NewKV("Required", strings.Join(names, ", ")))
}

func NewErrTooManyArgs(max, actual int) error {
return errors.New(errTooManyArgs, errors.NewKV("Max", max), errors.NewKV("Actual", actual))
}

func NewFailedToReadFile(inner error) error {
Expand Down
8 changes: 3 additions & 5 deletions cli/schema_migration_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,11 @@ Example:
defradb client schema migration get'

Learn more about the DefraDB GraphQL Schema Language on https://docs.source.network.`,
Args: func(cmd *cobra.Command, args []string) error {
RunE: func(cmd *cobra.Command, args []string) (err error) {
if err := cobra.NoArgs(cmd, args); err != nil {
return errors.New("this command take no arguments")
return NewErrTooManyArgs(0, len(args))
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) (err error) {

endpoint, err := httpapi.JoinPaths(cfg.API.AddressToURL(), httpapi.SchemaMigrationPath)
if err != nil {
return errors.Wrap("join paths failed", err)
Expand Down
23 changes: 12 additions & 11 deletions cli/schema_migration_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ Example: add from stdin:
cat schema_migration.lens | defradb client schema migration set bae123 bae456 -

Learn more about the DefraDB GraphQL Schema Language on https://docs.source.network.`,
Args: func(cmd *cobra.Command, args []string) error {
RunE: func(cmd *cobra.Command, args []string) (err error) {
if err := cobra.MinimumNArgs(2)(cmd, args); err != nil {
return errors.New("must specify src and dst schema versions, as well as a lens cfg")
return NewErrMissingArgs([]string{"src", "dst", "cfg"})
}
if err := cobra.MaximumNArgs(3)(cmd, args); err != nil {
return errors.New("must specify src and dst schema versions, as well as a lens cfg")
return NewErrTooManyArgs(3, len(args))
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) (err error) {

var lensCfgJson string
var srcSchemaVersionID string
var dstSchemaVersionID string
Expand All @@ -72,7 +70,7 @@ Learn more about the DefraDB GraphQL Schema Language on https://docs.source.netw
} else if len(args) == 2 {
// If the lensFile flag has not been provided then it must be provided as an arg
// and thus len(args) cannot be 2
return errors.Wrap("must provide a lens cfg", err)
return NewErrMissingArg("cfg")
} else if isFileInfoPipe(fi) && args[2] != "-" {
log.FeedbackInfo(
cmd.Context(),
Expand All @@ -98,17 +96,20 @@ Learn more about the DefraDB GraphQL Schema Language on https://docs.source.netw
dstSchemaVersionID = args[1]

if lensCfgJson == "" {
return errors.New("empty lens configuration provided")
return NewErrMissingArg("cfg")
}
if srcSchemaVersionID == "" {
return errors.New("no source schema version id provided")
return NewErrMissingArg("src")
}
if dstSchemaVersionID == "" {
return errors.New("no destination schema version id provided")
return NewErrMissingArg("dst")
}

decoder := json.NewDecoder(strings.NewReader(lensCfgJson))
decoder.DisallowUnknownFields()

var lensCfg model.Lens
err = json.Unmarshal([]byte(lensCfgJson), &lensCfg)
err = decoder.Decode(&lensCfg)
if err != nil {
return errors.Wrap("invalid lens configuration", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cli/schema_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ To learn more about the DefraDB GraphQL Schema Language, refer to https://docs.s
if err = cmd.Usage(); err != nil {
return err
}
return ErrTooManyArgs
return NewErrTooManyArgs(1, len(args))
}

if patchFile != "" {
Expand Down
110 changes: 110 additions & 0 deletions tests/integration/cli/client_schema_migration_get_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2023 Democratized Data Foundation
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package clitest

import (
"fmt"
"testing"

"github.com/sourcenetwork/defradb/tests/lenses"
)

func TestSchemaMigrationGet_GivenOneArg_ShouldReturnError(t *testing.T) {
conf := NewDefraNodeDefaultConfig(t)
stopDefra := runDefraNode(t, conf)

_, stderr := runDefraCommand(t, conf, []string{
"client", "schema", "migration", "get",
"notAnArg",
})
_ = stopDefra()

assertContainsSubstring(t, stderr, "too many arguments. Max: 0, Actual: 1")
}

func TestSchemaMigrationGet_GivenNoMigrations_ShouldSucceed(t *testing.T) {
conf := NewDefraNodeDefaultConfig(t)
stopDefra := runDefraNode(t, conf)

stdout, _ := runDefraCommand(t, conf, []string{
"client", "schema", "migration", "get",
})
_ = stopDefra()

assertContainsSubstring(t, stdout, `{"data":{"configuration":[]}}`)
}

func TestSchemaMigrationGet_GivenEmptyMigrationObj_ShouldSucceed(t *testing.T) {
islamaliev marked this conversation as resolved.
Show resolved Hide resolved
conf := NewDefraNodeDefaultConfig(t)
stopDefra := runDefraNode(t, conf)

stdout, _ := runDefraCommand(t, conf, []string{
"client", "schema", "migration", "set",
"bae123", "bae456", "{}",
})
assertContainsSubstring(t, stdout, "success")

stdout, _ = runDefraCommand(t, conf, []string{
"client", "schema", "migration", "get",
})
_ = stopDefra()

assertContainsSubstring(t, stdout,
`{"data":{"configuration":[{"SourceSchemaVersionID":"bae123","DestinationSchemaVersionID":"bae456","Lenses":null}]}}`,
)
}

func TestSchemaMigrationGet_GivenEmptyMigration_ShouldSucceed(t *testing.T) {
conf := NewDefraNodeDefaultConfig(t)
stopDefra := runDefraNode(t, conf)

stdout, _ := runDefraCommand(t, conf, []string{
"client", "schema", "migration", "set",
"bae123", "bae456", `{"lenses": []}`,
})
assertContainsSubstring(t, stdout, "success")

stdout, _ = runDefraCommand(t, conf, []string{
"client", "schema", "migration", "get",
})
_ = stopDefra()

assertContainsSubstring(t, stdout,
`{"data":{"configuration":[{"SourceSchemaVersionID":"bae123","DestinationSchemaVersionID":"bae456","Lenses":[]}]}}`,
)
}

func TestSchemaMigrationGet_GivenMigration_ShouldSucceed(t *testing.T) {
conf := NewDefraNodeDefaultConfig(t)
stopDefra := runDefraNode(t, conf)

stdout, _ := runDefraCommand(t, conf, []string{
"client", "schema", "migration", "set",
"bae123", "bae456",
fmt.Sprintf(`{"lenses": [{"path":"%s","arguments":{"dst":"verified","value":true}}]}`, lenses.SetDefaultModulePath),
})
assertContainsSubstring(t, stdout, "success")

stdout, _ = runDefraCommand(t, conf, []string{
"client", "schema", "migration", "get",
})
_ = stopDefra()

assertContainsSubstring(t, stdout,
`{"data":{"configuration":[{"SourceSchemaVersionID":"bae123","DestinationSchemaVersionID":"bae456","Lenses":[`+
fmt.Sprintf(
`{"Path":"%s",`,
lenses.SetDefaultModulePath,
)+
`"Inverse":false,"Arguments":{"dst":"verified","value":true}}`+
`]}]}}`,
)
}
Loading