From 9d861b54700464ab71f3077edaeff7cf2d703309 Mon Sep 17 00:00:00 2001 From: Tocho Tochev Date: Sun, 11 Feb 2024 20:59:56 +0200 Subject: [PATCH 1/2] chore: simplify test output closes#251 --- internal/storetest/testresult.go | 106 +++++++++++++++++++------------ 1 file changed, 64 insertions(+), 42 deletions(-) diff --git a/internal/storetest/testresult.go b/internal/storetest/testresult.go index 1402baa..260e911 100644 --- a/internal/storetest/testresult.go +++ b/internal/storetest/testresult.go @@ -59,7 +59,7 @@ func (result TestResult) IsPassing() bool { } //nolint:cyclop -func (result TestResult) FriendlyDisplay() string { +func (result TestResult) FriendlyFailuresDisplay() string { totalCheckCount := len(result.CheckResults) failedCheckCount := 0 totalListObjectsCount := len(result.ListObjectsResults) @@ -71,16 +71,7 @@ func (result TestResult) FriendlyDisplay() string { for index := 0; index < totalCheckCount; index++ { checkResult := result.CheckResults[index] - if checkResult.IsPassing() { - checkResultsOutput = fmt.Sprintf( - "%s\n✓ Check(user=%s,relation=%s,object=%s, context=%v)", - checkResultsOutput, - checkResult.Request.User, - checkResult.Request.Relation, - checkResult.Request.Object, - checkResult.Request.Context, - ) - } else { + if !checkResult.IsPassing() { failedCheckCount++ got := "N/A" @@ -107,16 +98,7 @@ func (result TestResult) FriendlyDisplay() string { for index := 0; index < totalListObjectsCount; index++ { listObjectsResult := result.ListObjectsResults[index] - if listObjectsResult.IsPassing() { - listObjectsResultsOutput = fmt.Sprintf( - "%s\n✓ ListObjects(user=%s,relation=%s,type=%s, context=%v)", - listObjectsResultsOutput, - listObjectsResult.Request.User, - listObjectsResult.Request.Relation, - listObjectsResult.Request.Type, - listObjectsResult.Request.Context, - ) - } else { + if !listObjectsResult.IsPassing() { failedListObjectsCount++ got := "N/A" @@ -139,30 +121,30 @@ func (result TestResult) FriendlyDisplay() string { } } - testStatus := "PASSING" if failedCheckCount+failedListObjectsCount != 0 { - testStatus = "FAILING" - } - - output := fmt.Sprintf( - "(%s) %s: Checks (%d/%d passing) | ListObjects (%d/%d passing)", - testStatus, - result.Name, - totalCheckCount-failedCheckCount, - totalCheckCount, - totalListObjectsCount-failedListObjectsCount, - totalListObjectsCount, - ) + testStatus := "FAILING" + output := fmt.Sprintf( + "(%s) %s: Checks (%d/%d passing) | ListObjects (%d/%d passing)", + testStatus, + result.Name, + totalCheckCount-failedCheckCount, + totalCheckCount, + totalListObjectsCount-failedListObjectsCount, + totalListObjectsCount, + ) + + if failedCheckCount > 0 { + output = fmt.Sprintf("%s%s", output, checkResultsOutput) + } - if failedCheckCount > 0 { - output = fmt.Sprintf("%s%s", output, checkResultsOutput) - } + if failedListObjectsCount > 0 { + output = fmt.Sprintf("%s%s", output, listObjectsResultsOutput) + } - if failedListObjectsCount > 0 { - output = fmt.Sprintf("%s%s", output, listObjectsResultsOutput) + return output } - return output + return "" } type TestResults struct { @@ -184,8 +166,48 @@ func (test TestResults) FriendlyDisplay() string { friendlyResults := []string{} for index := 0; index < len(test.Results); index++ { - friendlyResults = append(friendlyResults, test.Results[index].FriendlyDisplay()) + if !test.Results[index].IsPassing() { + friendlyResults = append(friendlyResults, test.Results[index].FriendlyFailuresDisplay()) + } + } + + failuresText := strings.Join(friendlyResults, "\n---\n") + + totalTestCount := len(test.Results) + failedTestCount := 0 + totalCheckCount := 0 + failedCheckCount := 0 + totalListObjectsCount := 0 + failedListObjectsCount := 0 + + for _, testResult := range test.Results { + if !testResult.IsPassing() { + failedTestCount++ + } + + totalCheckCount += len(testResult.CheckResults) + + for _, checkResult := range testResult.CheckResults { + if !checkResult.IsPassing() { + failedCheckCount++ + } + } + + totalListObjectsCount += len(testResult.ListObjectsResults) + + for _, listObjectsResult := range testResult.ListObjectsResults { + if !listObjectsResult.IsPassing() { + failedListObjectsCount++ + } + } } - return strings.Join(friendlyResults, "\n---\n") + summary := fmt.Sprintf( + "\n---\n\n# Test Summary #\nTests %d/%d passing\nChecks %d/%d passing\nListObjects %d/%d passing", + totalTestCount-failedTestCount, totalTestCount, + totalCheckCount-failedCheckCount, totalCheckCount, + totalListObjectsCount-failedListObjectsCount, totalListObjectsCount, + ) + + return failuresText + summary } From 8262cb3d06caf464d47bc5fa7e16eeca24f1960b Mon Sep 17 00:00:00 2001 From: Raghd Hamzeh Date: Mon, 12 Feb 2024 15:57:42 -0500 Subject: [PATCH 2/2] chore: fix newly found linting errors --- cmd/man.go | 2 +- cmd/model/get.go | 2 +- cmd/model/list.go | 2 +- cmd/model/test.go | 2 +- cmd/model/validate_test.go | 2 ++ cmd/store/create.go | 3 ++- cmd/store/delete.go | 2 +- cmd/store/get.go | 2 +- cmd/store/import.go | 4 +++- cmd/store/list.go | 2 +- cmd/tuple/changes.go | 2 +- cmd/tuple/import.go | 2 +- cmd/tuple/read.go | 2 +- cmd/tuple/write.go | 3 ++- cmd/version.go | 2 +- internal/cmdutils/get-contextual-tuples_test.go | 3 ++- internal/confirmation/confirmation_test.go | 2 ++ 17 files changed, 24 insertions(+), 15 deletions(-) diff --git a/cmd/man.go b/cmd/man.go index c2809e9..5ed57bf 100644 --- a/cmd/man.go +++ b/cmd/man.go @@ -32,7 +32,7 @@ var manCmd = &cobra.Command{ DisableFlagsInUseLine: true, Hidden: true, Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { manPage, err := mcobra.NewManPage(1, rootCmd.Root()) if err != nil { return err //nolint:wrapcheck diff --git a/cmd/model/get.go b/cmd/model/get.go index 617e423..c8c9d72 100644 --- a/cmd/model/get.go +++ b/cmd/model/get.go @@ -73,7 +73,7 @@ var getCmd = &cobra.Command{ Short: "Read a Single Authorization Model", Long: "Read an authorization model, pass in an empty model ID to get latest.", Example: `fga model get --store-id=01H0H015178Y2V4CX10C2KGHF4 --model-id=01GXSA8YR785C4FYS3C0RTG7B1`, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() diff --git a/cmd/model/list.go b/cmd/model/list.go index 249458e..76ee619 100644 --- a/cmd/model/list.go +++ b/cmd/model/list.go @@ -71,7 +71,7 @@ var listCmd = &cobra.Command{ Short: "Read Authorization Models", Long: "List authorization models in a store.", Example: "fga model list --store-id=01H0H015178Y2V4CX10C2KGHF4", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() if err != nil { diff --git a/cmd/model/test.go b/cmd/model/test.go index c721134..606b925 100644 --- a/cmd/model/test.go +++ b/cmd/model/test.go @@ -34,7 +34,7 @@ var testCmd = &cobra.Command{ Short: "Test an Authorization Model", Long: "Run a set of tests against a particular Authorization Model.", Example: `fga model test --tests model.fga.yaml`, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() diff --git a/cmd/model/validate_test.go b/cmd/model/validate_test.go index f69e128..2636a2e 100644 --- a/cmd/model/validate_test.go +++ b/cmd/model/validate_test.go @@ -83,10 +83,12 @@ func TestValidate(t *testing.T) { t.Parallel() model := authorizationmodel.AuthzModel{} + err := model.ReadFromJSONString(test.Input) if err != nil { return } + output := validate(model) if !reflect.DeepEqual(output, test.ExpectedOutput) { diff --git a/cmd/store/create.go b/cmd/store/create.go index 3984da2..65cde72 100644 --- a/cmd/store/create.go +++ b/cmd/store/create.go @@ -18,6 +18,7 @@ package store import ( "context" + "errors" "fmt" "github.com/openfga/go-sdk/client" @@ -60,7 +61,7 @@ func CreateStoreWithModel( response := CreateStoreAndModelResponse{} if storeName == "" { - return nil, fmt.Errorf(`required flag(s) "name" not set`) //nolint:goerr113 + return nil, errors.New(`required flag(s) "name" not set`) //nolint:goerr113 } createStoreResponse, err := create(fgaClient, storeName) diff --git a/cmd/store/delete.go b/cmd/store/delete.go index a37fdc4..ecfda64 100644 --- a/cmd/store/delete.go +++ b/cmd/store/delete.go @@ -34,7 +34,7 @@ var deleteCmd = &cobra.Command{ Short: "Delete Store", Long: "Mark a store as deleted.", Example: "fga store delete --store-id=01H0H015178Y2V4CX10C2KGHF4", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) // First, confirm whether this is intended force, err := cmd.Flags().GetBool("force") diff --git a/cmd/store/get.go b/cmd/store/get.go index 2e57627..538b891 100644 --- a/cmd/store/get.go +++ b/cmd/store/get.go @@ -44,7 +44,7 @@ var getCmd = &cobra.Command{ Short: "Get Store", Long: `Get a particular store.`, Example: "fga store get --store-id=01H0H015178Y2V4CX10C2KGHF4", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() if err != nil { diff --git a/cmd/store/import.go b/cmd/store/import.go index 995e381..c56f91d 100644 --- a/cmd/store/import.go +++ b/cmd/store/import.go @@ -52,11 +52,13 @@ func importStore( } else { authModel := authorizationmodel.AuthzModel{} clientConfig.StoreID = storeID + if format == authorizationmodel.ModelFormatJSON { err = authModel.ReadFromJSONString(storeData.Model) } else { err = authModel.ReadFromDSLString(storeData.Model) } + if err != nil { return err //nolint:wrapcheck } @@ -90,7 +92,7 @@ var importCmd = &cobra.Command{ Short: "Import Store Data", Long: `Import a store: updating the name, model and appending the global tuples`, Example: "fga store import --file=model.fga.yaml", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) storeID, err := cmd.Flags().GetString("store-id") diff --git a/cmd/store/list.go b/cmd/store/list.go index 0ef4fa7..8904178 100644 --- a/cmd/store/list.go +++ b/cmd/store/list.go @@ -65,7 +65,7 @@ var listCmd = &cobra.Command{ Short: "List Stores", Long: `Get a list of stores.`, Example: "fga store list", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() if err != nil { diff --git a/cmd/tuple/changes.go b/cmd/tuple/changes.go index 75df7fa..4236d08 100644 --- a/cmd/tuple/changes.go +++ b/cmd/tuple/changes.go @@ -71,7 +71,7 @@ var changesCmd = &cobra.Command{ Short: "Read Relationship Tuple Changes (Watch)", Long: "Get a list of relationship tuple changes (Writes and Deletes) across time.", Example: "fga tuple changes --store-id=01H0H015178Y2V4CX10C2KGHF4 --type document --continuation-token=MXw=", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() diff --git a/cmd/tuple/import.go b/cmd/tuple/import.go index ff41b5e..10c88aa 100644 --- a/cmd/tuple/import.go +++ b/cmd/tuple/import.go @@ -138,7 +138,7 @@ var importCmd = &cobra.Command{ Deprecated: "use the write/delete command with the flag --file instead", Long: "Imports Relationship Tuples to the store. " + "This will write the tuples in chunks and at the end will report the tuple chunks that failed.", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() diff --git a/cmd/tuple/read.go b/cmd/tuple/read.go index 2f171e4..175d0f9 100644 --- a/cmd/tuple/read.go +++ b/cmd/tuple/read.go @@ -103,7 +103,7 @@ var readCmd = &cobra.Command{ Short: "Read Relationship Tuples", Long: "Read relationship tuples that exist in the system (does not evaluate).", Example: "fga tuple read --store-id=01H0H015178Y2V4CX10C2KGHF4 --user user:anne --relation can_view --object document:roadmap", //nolint:lll - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() diff --git a/cmd/tuple/write.go b/cmd/tuple/write.go index 0a4af86..3850aaf 100644 --- a/cmd/tuple/write.go +++ b/cmd/tuple/write.go @@ -18,6 +18,7 @@ package tuple import ( "context" + "errors" "fmt" "github.com/openfga/go-sdk/client" @@ -114,7 +115,7 @@ func writeTuplesFromFile(flags *flag.FlagSet, fgaClient *client.OpenFgaClient) e } if fileName == "" { - return fmt.Errorf("file name cannot be empty") //nolint:goerr113 + return errors.New("file name cannot be empty") //nolint:goerr113 } maxTuplesPerWrite, err := flags.GetInt("max-tuples-per-write") diff --git a/cmd/version.go b/cmd/version.go index 02e6cc8..ed368ca 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -31,7 +31,7 @@ var versionCmd *cobra.Command = &cobra.Command{ Use: "version", Short: "Reports the FGA CLI version", Long: "Reports the FGA CLI version.", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { fmt.Printf("fga version %s\n", versionStr) return nil diff --git a/internal/cmdutils/get-contextual-tuples_test.go b/internal/cmdutils/get-contextual-tuples_test.go index d952f53..1f064b4 100644 --- a/internal/cmdutils/get-contextual-tuples_test.go +++ b/internal/cmdutils/get-contextual-tuples_test.go @@ -65,6 +65,7 @@ func TestGetContextualTuplesWithNoError(t *testing.T) { test := tests[index] t.Run("TestGetContextualTuplesWithNoError"+string(rune(index)), func(t *testing.T) { t.Parallel() + tuples, err := cmdutils.ParseContextualTuplesInner(test.raw) if err != nil { t.Error(err) @@ -109,8 +110,8 @@ func TestGetContextualTuplesWithError(t *testing.T) { test := tests[index] t.Run("TestGetContextualTuplesWithNoError"+string(rune(index)), func(t *testing.T) { t.Parallel() - _, err := cmdutils.ParseContextualTuplesInner(test.raw) + _, err := cmdutils.ParseContextualTuplesInner(test.raw) if err == nil { t.Error("Expect error but there is none") } diff --git a/internal/confirmation/confirmation_test.go b/internal/confirmation/confirmation_test.go index 61e093e..3db1978 100644 --- a/internal/confirmation/confirmation_test.go +++ b/internal/confirmation/confirmation_test.go @@ -52,10 +52,12 @@ func TestConfirmation(t *testing.T) { test := test t.Run(test._name, func(t *testing.T) { t.Parallel() + result, err := askForConfirmation(bufio.NewReader(strings.NewReader(test.input)), "test") if err != nil { t.Error(err) } + if result != test.result { t.Errorf("Expect result %v actual %v", test.result, result) }