Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pierrebeaucamp committed Feb 13, 2025
1 parent b529dd4 commit 6f04a27
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 109 deletions.
80 changes: 0 additions & 80 deletions cmd/captain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"context"
"encoding/json"
"fmt"
"os"
"path"
Expand All @@ -11,14 +10,11 @@ import (

"github.com/caarlos0/env/v7"
"github.com/spf13/cobra"
"go.uber.org/zap"
"gopkg.in/yaml.v3"

"github.com/rwx-research/captain-cli/internal/backend/remote"
"github.com/rwx-research/captain-cli/internal/cli"
"github.com/rwx-research/captain-cli/internal/errors"
"github.com/rwx-research/captain-cli/internal/providers"
v1 "github.com/rwx-research/captain-cli/internal/testingschema/v1"
)

// Config is the internal representation of the configuration.
Expand Down Expand Up @@ -218,79 +214,3 @@ func InitConfig(cmd *cobra.Command, cliArgs CliArgs) (cfg Config, err error) {

return cfg, nil
}

func getRecipes(logger *zap.SugaredLogger, cfg Config) (map[string]v1.TestIdentityRecipe, error) {
var recipeBuffer []byte
var newRecipesFile bool

existingCaptainDir, err := findInParentDir(captainDirectory)
if err != nil {
if !errors.Is(err, os.ErrNotExist) {
return nil, err
}

if err = os.Mkdir(captainDirectory, 0o755); err != nil {
return nil, errors.WithStack(err)
}

existingCaptainDir = captainDirectory
}

recipesFile, err := findInParentDir(filepath.Join(captainDirectory, "recipes.json"))
if err == nil {
recipeBuffer, err = os.ReadFile(recipesFile)
}
if err != nil {
if recipesFile == "" {
newRecipesFile = true
recipesFile = filepath.Join(existingCaptainDir, "recipes.json")
}

client, err := remote.NewClient(remote.ClientConfig{
Debug: cfg.Output.Debug,
Host: cfg.Cloud.APIHost,
Insecure: cfg.Cloud.Insecure,
Log: logger,
Token: "none", // Can't be empty. We rely on implementation details here that `GetIdentityRecipes` will not use it
})
if err != nil {
return nil, errors.Wrap(err, "Unable to initialize API client")
}

recipeBuffer, err = client.GetIdentityRecipes(context.Background())
if err != nil {
return nil, errors.Wrap(err, "Unable to fetch test identity recipes from API")
}
}

type IdentityRecipe struct {
Language string
Kind string
Recipe struct {
Components []string
Strict bool
}
}

recipeList := []IdentityRecipe{}
if err := json.Unmarshal(recipeBuffer, &recipeList); err != nil {
return nil, errors.NewInternalError("unable to parse identiy recipes: %s", err.Error())
}

recipes := make(map[string]v1.TestIdentityRecipe)
for _, identityRecipe := range recipeList {
recipes[v1.CoerceFramework(identityRecipe.Language, identityRecipe.Kind).String()] = v1.TestIdentityRecipe{
Components: identityRecipe.Recipe.Components,
Strict: identityRecipe.Recipe.Strict,
}
}

// Do this last so we can be sure that the buffer can actually be parsed correctly
if newRecipesFile {
if err = os.WriteFile(recipesFile, recipeBuffer, 0o600); err != nil {
logger.Warnf("unable to cache identity recipes on disk: %s", err.Error())
}
}

return recipes, nil
}
131 changes: 131 additions & 0 deletions cmd/captain/identity_recipes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package main

import (
"context"
"encoding/json"
"os"
"path/filepath"

"go.uber.org/zap"

"github.com/rwx-research/captain-cli"
"github.com/rwx-research/captain-cli/internal/backend/remote"
"github.com/rwx-research/captain-cli/internal/errors"
v1 "github.com/rwx-research/captain-cli/internal/testingschema/v1"
)

type IdentityRecipe struct {
Language string
Kind string
Recipe struct {
Components []string
Strict bool
}
}

type Cache struct {
CaptainVersion string
Recipes []IdentityRecipe
}

func getRecipes(logger *zap.SugaredLogger, cfg Config) (map[string]v1.TestIdentityRecipe, error) {
var cache Cache

existingCaptainDir, err := findInParentDir(captainDirectory)
if err != nil {
if !errors.Is(err, os.ErrNotExist) {
return nil, err
}

if err = os.Mkdir(captainDirectory, 0o755); err != nil {
return nil, errors.WithStack(err)
}

existingCaptainDir = captainDirectory
}

recipesFile, err := findInParentDir(filepath.Join(captainDirectory, "recipes.json"))
if err == nil {
// Fall back to getting recipes from Cloud on error
cache, err = getRecipesFromCache(recipesFile)
}
if err != nil {
cache, err = getRecipesFromCloud(logger, cfg)
if err != nil {
return nil, errors.WithStack(err)
}

if recipesFile == "" {
recipesFile = filepath.Join(existingCaptainDir, "recipes.json")
}

// Write to cache file
buffer, err := json.Marshal(cache)
if err == nil {
err = os.WriteFile(recipesFile, buffer, 0o600)
}
if err != nil {
logger.Warnf("unable to cache identity recipes on disk: %s", err.Error())
}
}

recipes := make(map[string]v1.TestIdentityRecipe)
for _, identityRecipe := range cache.Recipes {
recipes[v1.CoerceFramework(identityRecipe.Language, identityRecipe.Kind).String()] = v1.TestIdentityRecipe{
Components: identityRecipe.Recipe.Components,
Strict: identityRecipe.Recipe.Strict,
}
}

return recipes, nil
}

func getRecipesFromCache(filePath string) (Cache, error) {
var buffer []byte
var cache Cache

buffer, err := os.ReadFile(filePath)
if err != nil {
return Cache{}, errors.WithStack(err)
}

if err = json.Unmarshal(buffer, &cache); err != nil {
return Cache{}, errors.WithStack(err)
}

if cache.CaptainVersion != captain.Version {
return Cache{}, errors.NewCacheError("Outdated Captain Version")
}

return cache, nil
}

func getRecipesFromCloud(logger *zap.SugaredLogger, cfg Config) (Cache, error) {
var buffer []byte
var recipeList []IdentityRecipe

client, err := remote.NewClient(remote.ClientConfig{
Debug: cfg.Output.Debug,
Host: cfg.Cloud.APIHost,
Insecure: cfg.Cloud.Insecure,
Log: logger,
Token: "none", // Can't be empty. We rely on implementation details here that `GetIdentityRecipes` will not use it
})
if err != nil {
return Cache{}, errors.Wrap(err, "Unable to initialize API client")
}

buffer, err = client.GetIdentityRecipes(context.Background())
if err != nil {
return Cache{}, errors.Wrap(err, "Unable to fetch test identity recipes from API")
}

if err = json.Unmarshal(buffer, &recipeList); err != nil {
return Cache{}, errors.NewInternalError("unable to parse identiy recipes: %s", err.Error())
}

return Cache{
CaptainVersion: captain.Version,
Recipes: recipeList,
}, nil
}
24 changes: 23 additions & 1 deletion internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,31 @@ func NewRetryError(msg string, a ...any) error {
return WithStack(RetryError{errors.Errorf(msg, a...)})
}

// AsRetryError checks whether the error is an internal error
// AsRetryError checks whether the error is a retry error
func AsRetryError(err error) (RetryError, bool) {
var e RetryError
ok := As(err, &e)
return e, ok
}

// CacheError is returned when the CLI encountered a cache error. It's intended that the application catches these errors and
// handles them gracefully
type CacheError struct {
E error
}

func (e CacheError) Error() string {
return e.E.Error()
}

// CacheError returns a new CacheError
func NewCacheError(msg string, a ...any) error {
return WithStack(CacheError{errors.Errorf(msg, a...)})
}

// AsCacheError checks whether the error is a cache error
func AsCacheError(err error) (CacheError, bool) {
var e CacheError
ok := As(err, &e)
return e, ok
}
39 changes: 11 additions & 28 deletions internal/parsing/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/base64"
"fmt"
"io"
"reflect"

"go.uber.org/zap"

Expand Down Expand Up @@ -141,7 +140,7 @@ func parseWith(file fs.File, parsers []Parser, groupNumber int, cfg Config) (*v1
finalResults := parsedTestResults[0]
cfg.Logger.Debugf("%T was ultimately responsible for parsing the test results", firstParser)

testIDsAreUnique, err := checkIfTestIDsAreUnique(firstParser, finalResults.Tests, cfg)
testIDsAreUnique, err := checkIfTestIDsAreUnique(finalResults, cfg)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -182,42 +181,26 @@ func parseWith(file fs.File, parsers []Parser, groupNumber int, cfg Config) (*v1
return &finalResults, nil
}

func checkIfTestIDsAreUnique(parser Parser, tests []v1.Test, cfg Config) (bool, error) {
var identityRecipe *v1.TestIdentityRecipe
func checkIfTestIDsAreUnique(testResult v1.TestResults, cfg Config) (bool, error) {
uniqueTestIdentifiers := make(map[string]struct{})

FRAMEWORKS:
for framework, frameworkParsers := range cfg.FrameworkParsers {
for _, frameworkParser := range frameworkParsers {
if reflect.TypeOf(frameworkParser) != reflect.TypeOf(parser) {
continue
}

// TODO: What about "other" frameworks (i.e. when we don't match here)
if recipe, ok := cfg.IdentityRecipes[framework.String()]; ok {
identityRecipe = &recipe
}

break FRAMEWORKS
}
}
identityRecipe, recipeFound := cfg.IdentityRecipes[testResult.Framework.String()]

// Populate `uniqueTestIdentifiers` with the ID of the last test as the for-loop that follows will
// not cover it.
if identityRecipe != nil {
id, err := tests[len(tests)-1].Identify(*identityRecipe)
if recipeFound {
id, err := testResult.Tests[len(testResult.Tests)-1].Identify(identityRecipe)
if err != nil {
return false, errors.Wrap(err, "Unable to construct identity from test")
}

uniqueTestIdentifiers[id] = struct{}{}
}

for i := 0; i < len(tests)-1; i++ {
test := tests[i]
for i := 0; i < len(testResult.Tests)-1; i++ {
test := testResult.Tests[i]

if identityRecipe != nil {
id, err := test.Identify(*identityRecipe)
if recipeFound {
id, err := test.Identify(identityRecipe)
if err != nil {
return false, errors.Wrap(err, "Unable to construct identity from test")
}
Expand All @@ -229,8 +212,8 @@ FRAMEWORKS:
uniqueTestIdentifiers[id] = struct{}{}
}

for j := i + 1; j < len(tests); j++ {
if test.Matches(tests[j]) {
for j := i + 1; j < len(testResult.Tests); j++ {
if test.Matches(testResult.Tests[j]) {
return false, nil
}
}
Expand Down

0 comments on commit 6f04a27

Please sign in to comment.