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

Added test cases for Delete #8

Merged
merged 58 commits into from
Jul 13, 2022
Merged

Conversation

kotewar
Copy link
Contributor

@kotewar kotewar commented Jun 29, 2022

Overview

  • Aim for this PR is to write unit tests of Delete CLI.
  • Update Error handing to support better tests

Changes

  • Added tests for all the delete.go

Pending items

  • Figure out how to take user input for delete without confirm flag

Base automatically changed from t-dedah/tests-for-list to main July 13, 2022 20:44
Copy link
Contributor

@t-dedah t-dedah left a comment

Choose a reason for hiding this comment

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

Approving PR to move forward.@kotewar will raise a new PR with few fixes from bugbash and pending comments

Raised an issue to track the pending comment - #13

@@ -44,30 +49,46 @@ func NewCmdDelete() *cobra.Command {
if !f.Confirm {
matchedCaches, err := getCacheListWithExactMatch(f, artifactCache)
if err != nil {
return err
var httpError api.HTTPError
Copy link
Contributor

Choose a reason for hiding this comment

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

This error handling is exactly same with list.go so abstract this out to a separate function

f.Confirm = choice == "Delete"
fmt.Println()
}
if f.Confirm {
cachesDeleted, err := artifactCache.DeleteCaches(queryParams)
if err != nil {
return err
var httpError api.HTTPError
Copy link
Contributor

Choose a reason for hiding this comment

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

This can also be handled using the same error handling function proposed above.

@t-dedah
Copy link
Contributor

t-dedah commented Jul 13, 2022

Merging this now and expect another PR to resolve comments. This is being done to not overload this PR and also close the long running PR.

@t-dedah t-dedah merged commit 790f19d into main Jul 13, 2022
@t-dedah t-dedah deleted the users/kotewar/error-handling-and-tests branch July 13, 2022 21:03
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.75 * (width - 15 - 20)))
refWidth := int(math.Floor(0.25 * (width - 15 - 20)))
keyWidth := int(math.Floor(0.65 * float64(width-15-20)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use consts SIZE_COLUMN_WIDTH and LAST_ACCESSED_AT_COLUMN_WIDTH instead of hard-coded values. I missed it in my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants