actions/gh-actions-cache
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bugbash fixes and refactoring (#14)

Browse Source 
* Completed List cmd and added API calls

* Minor comments and add delete code to pass linting

* Typo in descriptions

* Minor comments

* Validations

* Validations-1

* improved branch flag validation

* removed build

* working after refactory with bad names

* Command working, test not working

* Corrected creation of service

* Finalized structure using service

* Deleted tests

* cleanup

* cleanup

* cleanup

* removed space with tab

* aligned types in model.go

* Update model.go

* resolved comments

* Refactor

* removed long descriptions

* Working incomplete tests

* Completed tests

* cleanup

* checks

* PR comments

* PR comments

* minor comment issue

* minor comment issue

* Added test cases for Delete

* updated tests to work with workflow

* Added missing error condition

* Updated tests to support new option service

* Improved eror handling for list

* Fixed test case

* Improved error handling

* Error handling and test cases for delete API calls

* Added test case for user confirmation delete.

* Removed unused import from test

* Fixed test case for error scenario

* Upgraded go-gh

* reusing rest client error

* Fix for failing windows test cases

* help cmd removed when cache isnt present on delete

* Pretty print ratio and space between cols modified

* Error handling wrapping

* Reverted back error message after silencing help

* Bugbash fixes for int limit, zero cache list msg

* Test case fixes for error message changes

* Handling no cache list scenario with(out) key

* Minor Refactoring and avoided code duplication

* Removed unused inputs after resolving conflicts

* Formatted test file

* removed err5xx as they all have same value.

* Removed err5xx from list as well

* Help and error message enhancements for list.

* changing commandname to avoid conflicts

* Ran `go fmt` formatter against all .go files

* Removed command from root.go

* Updated version to 1.0.0

Co-authored-by: t-dedah <t-dedah@github.com>
Co-authored-by: Deepak Dahiya <59823596+t-dedah@users.noreply.github.com>
This commit is contained in:
Sankalp Kotewar
2022-07-18 14:51:28 +05:30
committed by GitHub
parent 19af1838b9
commit 8365ebe619
10 changed files with 45 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
cmd/delete.go
View File

@@ -1,7 +1,6 @@
package cmd
import (
"errors"
"fmt"
"net/url"
"strings"
@@ -10,14 +9,13 @@ import (
"github.com/actions/gh-actions-cache/internal"
"github.com/actions/gh-actions-cache/service"
"github.com/actions/gh-actions-cache/types"
"github.com/cli/go-gh/pkg/api"
"github.com/spf13/cobra"
)
var choice string = ""
func NewCmdDelete() *cobra.Command {
COMMAND = "delete"
deleteCommand := "delete"
f := types.DeleteOptions{}
var deleteCmd = &cobra.Command{
@@ -38,7 +36,7 @@ func NewCmdDelete() *cobra.Command {
// This will silence the usage (help) message as they are not needed for errors beyond this point
cmd.SilenceUsage = true
artifactCache, err := service.NewArtifactCache(repo, COMMAND, VERSION)
artifactCache, err := service.NewArtifactCache(repo, deleteCommand, VERSION)
if err != nil {
return types.HandledError{Message: err.Error(), InnerError: err}
}
@@ -49,14 +47,7 @@ func NewCmdDelete() *cobra.Command {
if !f.Confirm {
matchedCaches, err := getCacheListWithExactMatch(f, artifactCache)
if err != nil {
var httpError api.HTTPError
if errors.As(err, &httpError) && httpError.StatusCode == 404 {
return types.HandledError{Message: "The given repo does not exist.", InnerError: err}
} else if errors.As(err, &httpError) && httpError.StatusCode >= 400 && httpError.StatusCode < 500 {
return types.HandledError{Message: httpError.Message, InnerError: err}
} else {
return types.HandledError{Message: "We could not process your request due to internal error.", InnerError: err}
}
return internal.HttpErrorHandler(err, "The given repo does not exist.")
}
matchedCachesLen := len(matchedCaches)
if matchedCachesLen == 0 {
@@ -81,14 +72,7 @@ func NewCmdDelete() *cobra.Command {
if f.Confirm {
cachesDeleted, err := artifactCache.DeleteCaches(queryParams)
if err != nil {
var httpError api.HTTPError
if errors.As(err, &httpError) && httpError.StatusCode == 404 {
return types.HandledError{Message: fmt.Sprintf("Cache with input key '%s' does not exist", f.Key), InnerError: err}
} else if errors.As(err, &httpError) && httpError.StatusCode >= 400 && httpError.StatusCode < 500 {
return types.HandledError{Message: httpError.Message, InnerError: err}
} else {
return types.HandledError{Message: "We could not process your request due to internal error.", InnerError: err}
}
return internal.HttpErrorHandler(err, fmt.Sprintf("Cache with input key '%s' does not exist", f.Key))
}
if cachesDeleted > 0 {

32
cmd/list.go
View File

@@ -1,23 +1,21 @@
package cmd
import (
"errors"
"fmt"
"net/url"
"github.com/actions/gh-actions-cache/internal"
"github.com/actions/gh-actions-cache/service"
"github.com/actions/gh-actions-cache/types"
"github.com/cli/go-gh/pkg/api"
"github.com/spf13/cobra"
)
func NewCmdList() *cobra.Command {
COMMAND = "list"
listCommand := "list"
f := types.ListOptions{}
var listCmd = &cobra.Command {
var listCmd = &cobra.Command{
Use: "list",
Short: "Lists the actions cache",
RunE: func(cmd *cobra.Command, args []string) error {
@@ -38,14 +36,14 @@ func NewCmdList() *cobra.Command {
return err
}
artifactCache, err := service.NewArtifactCache(repo, COMMAND, VERSION)
artifactCache, err := service.NewArtifactCache(repo, listCommand, VERSION)
if err != nil {
return types.HandledError{Message: err.Error(), InnerError: err}
}
if f.Branch == "" && f.Key == "" {
totalCacheSize, err := artifactCache.GetCacheUsage()
if err == nil {
if err == nil && totalCacheSize > 0 {
fmt.Printf("Total caches size %s\n\n", internal.FormatCacheSize(totalCacheSize))
}
}
@@ -54,28 +52,24 @@ func NewCmdList() *cobra.Command {
f.GenerateQueryParams(queryParams)
listCacheResponse, err := artifactCache.ListCaches(queryParams)
if err != nil {
var httpError api.HTTPError
if errors.As(err, &httpError) && httpError.StatusCode == 404 {
return types.HandledError{Message: "The given repo does not exist.", InnerError: err}
} else if errors.As(err, &httpError) && httpError.StatusCode >= 400 && httpError.StatusCode < 500 {
return types.HandledError{Message: httpError.Message, InnerError: err}
} else {
return types.HandledError{Message: "We could not process your request due to internal error.", InnerError: err}
}
return internal.HttpErrorHandler(err, "The given repo does not exist.")
}
totalCaches := listCacheResponse.TotalCount
caches := listCacheResponse.ActionsCaches
fmt.Printf("Showing %d of %d cache entries in %s/%s\n\n", displayedEntriesCount(len(caches), f.Limit), totalCaches, repo.Owner(), repo.Name())
internal.PrettyPrintCacheList(caches)
return nil
if len(caches) > 0 {
fmt.Printf("Showing %d of %d cache entries in %s/%s\n\n", displayedEntriesCount(len(caches), f.Limit), totalCaches, repo.Owner(), repo.Name())
internal.PrettyPrintCacheList(caches)
} else {
fmt.Printf("There are no Actions caches currently present in this repo or for the provided filters\n")
}
return nil
},
}
listCmd.Flags().StringVarP(&f.Repo, "repo", "R", "", "Select another repository for finding actions cache.")
listCmd.Flags().StringVarP(&f.Branch, "branch", "B", "", "Filter by branch")
listCmd.Flags().IntVarP(&f.Limit, "limit", "", 30, "Maximum number of items to fetch (default is 30, max limit is 100)")
listCmd.Flags().IntVarP(&f.Limit, "limit", "", 30, "Number of items to fetch between 1 and 100")
listCmd.Flags().StringVarP(&f.Key, "key", "", "", "Filter by key")
listCmd.Flags().StringVarP(&f.Order, "order", "", "", "Order of caches returned (asc/desc)")
listCmd.Flags().StringVarP(&f.Sort, "sort", "", "", "Sort fetched caches (last-used/size/created-at)")

10
cmd/list_test.go
View File

@@ -3,12 +3,14 @@ package cmd
import (
"errors"
"fmt"
"testing"
"reflect"
"testing"
"github.com/actions/gh-actions-cache/internal"
"github.com/stretchr/testify/assert"
"github.com/actions/gh-actions-cache/types"
"github.com/stretchr/testify/assert"
"gopkg.in/h2non/gock.v1"
)
@@ -44,7 +46,7 @@ func TestListWithNegativeLimit(t *testing.T) {
err := cmd.Execute()
assert.NotNil(t, err)
assert.Equal(t, err, fmt.Errorf("-1 is not a valid value for limit flag. Allowed values: 1-100"))
assert.Equal(t, err, fmt.Errorf("-1 is not a valid integer value for limit flag. Allowed values: 1-100"))
assert.True(t, gock.IsDone(), internal.PrintPendingMocks(gock.Pending()))
}
@@ -56,7 +58,7 @@ func TestListWithIncorrectLimit(t *testing.T) {
err := cmd.Execute()
assert.NotNil(t, err)
assert.Equal(t, err, fmt.Errorf("101 is not a valid value for limit flag. Allowed values: 1-100"))
assert.Equal(t, err, fmt.Errorf("101 is not a valid integer value for limit flag. Allowed values: 1-100"))
assert.True(t, gock.IsDone(), internal.PrintPendingMocks(gock.Pending()))
}

4
cmd/root.go
View File

@@ -6,9 +6,7 @@ import (
"github.com/spf13/cobra"
)
const VERSION = "0.0.1"
var COMMAND string = ""
const VERSION = "1.0.0"
var rootCmd = &cobra.Command{
Use: "gh-actions-cache",

2
internal/test_utils.go
View File

@@ -13,4 +13,4 @@ func PrintPendingMocks(mocks []gock.Mock) string {
paths = append(paths, mock.Request().URLStruct.String())
}
return fmt.Sprintf("%d unmatched mocks: %s", len(paths), strings.Join(paths, ", "))
}
}

19
internal/utils.go
View File

@@ -1,6 +1,7 @@
package internal
import (
"errors"
"fmt"
"math"
"os"
@@ -12,6 +13,7 @@ import (
"github.com/TwiN/go-color"
"github.com/actions/gh-actions-cache/types"
gh "github.com/cli/go-gh"
"github.com/cli/go-gh/pkg/api"
ghRepo "github.com/cli/go-gh/pkg/repository"
"github.com/cli/safeexec"
"github.com/mattn/go-isatty"
@@ -51,17 +53,17 @@ func FormatCacheSize(size_in_bytes float64) string {
func PrettyPrintCacheList(caches []types.ActionsCache) {
width, _, _ := getTerminalWidth(os.Stdout)
width = int(math.Max(float64(width), 100))
sizeWidth := SIZE_COLUMN_WIDTH // hard-coded size as the content is scoped
timeWidth := LAST_ACCESSED_AT_COLUMN_WIDTH // hard-coded size as the content is scoped
keyWidth := int(math.Floor(0.65 * float64(width-15-20)))
refWidth := int(math.Floor(0.20 * float64(width-15-20)))
keyWidth := int(math.Floor(0.65 * float64(width-SIZE_COLUMN_WIDTH-LAST_ACCESSED_AT_COLUMN_WIDTH)))
refWidth := int(math.Floor(0.20 * float64(width-SIZE_COLUMN_WIDTH-LAST_ACCESSED_AT_COLUMN_WIDTH)))
for _, cache := range caches {
var formattedRow string = getFormattedCacheInfo(cache, keyWidth, sizeWidth, refWidth, timeWidth)
fmt.Println(formattedRow)
}
}
func PrettyPrintTrimmedCacheList(caches []types.ActionsCache) {
length := len(caches)
limit := 30
@@ -127,3 +129,14 @@ func getTerminalWidth(f *os.File) (int, int, error) {
}
return 100, 100, nil
}
func HttpErrorHandler(err error, errMsg404 string) types.HandledError {
var httpError api.HTTPError
if errors.As(err, &httpError) && httpError.StatusCode == 404 {
return types.HandledError{Message: errMsg404, InnerError: err}
} else if errors.As(err, &httpError) && httpError.StatusCode >= 400 && httpError.StatusCode < 500 {
return types.HandledError{Message: httpError.Message, InnerError: err}
} else {
return types.HandledError{Message: "We could not process your request due to internal error.", InnerError: err}
}
}

2
internal/utils_test.go
View File

@@ -52,4 +52,4 @@ func TestFormatCacheSize_GB(t *testing.T) {
assert.NotNil(t, cacheSizeDetailString)
assert.Equal(t, cacheSizeDetailString, "1.50 GB")
}
}

2
service/actions_cache_test.go
View File

@@ -12,7 +12,7 @@ import (
"gopkg.in/h2non/gock.v1"
)
const VERSION string = "0.0.1"
const VERSION string = "1.0.0"
func TestGetCacheUsage_CorrectRepo(t *testing.T) {
t.Cleanup(gock.Off)

3
types/errors.go
View File

@@ -5,7 +5,7 @@ import (
)
type HandledError struct {
Message string
Message string
InnerError error
}
@@ -13,4 +13,3 @@ type HandledError struct {
func (err HandledError) Error() string {
return fmt.Sprintf(err.Message)
}

2
types/options.go
View File

@@ -41,7 +41,7 @@ func (o *ListOptions) Validate() error {
}
if o.Limit < 1 || o.Limit > 100 {
return fmt.Errorf(fmt.Sprintf("%d is not a valid value for limit flag. Allowed values: 1-100", o.Limit))
return fmt.Errorf(fmt.Sprintf("%d is not a valid integer value for limit flag. Allowed values: 1-100", o.Limit))
}
return nil