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.

Sign up for GitHub

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

Jump to bottom

fix: rename the Attempts field to Attempt; expose in gh run view and gh run ls #8905

Merged
merged 3 commits into from
May 22, 2024
Merged

fix: rename the Attempts field to Attempt; expose in gh run view and gh run ls #8905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed5690c
expose `run_attempt` when getting workflow runs
cawfeecake Apr 1, 2024
35653f4
Merge branch 'trunk' into expose-attempt
cawfeecake May 20, 2024
75c5589
Test that run attempt count is exportable
williammartin May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/cmd/run/shared/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ var RunFields = []string{
"createdAt",
"updatedAt",
"startedAt",
"attempt",
"status",
"conclusion",
"event",
Expand All @@ -97,7 +98,7 @@ type Run struct {
workflowName string // cache column
WorkflowID int64 `json:"workflow_id"`
Number int64 `json:"run_number"`
Attempts uint64 `json:"run_attempt"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm genuinely surprised there is no other code that uses this field since it was added in #5945 🤯

That said, how should test cases within the following be updated to ensure this is being exported?

cli/pkg/cmd/run/shared/shared_test.go

Line 111 in ed5690c

func TestRunExportData(t *testing.T) {

Copy link
Member

Choose a reason for hiding this comment

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

I'm genuinely surprised there is no other code that uses this field since it was added in #5945 🤯

I also went down this rabbit hole like gahhh. It's funny because it was even updated to use uint64 instead of 8 bits because it was causing an unmarshal error even tho it was never useeeeeed.

Attempt uint64 `json:"run_attempt"`
HeadBranch string `json:"head_branch"`
JobsURL string `json:"jobs_url"`
HeadCommit Commit `json:"head_commit"`
Expand Down
9 changes: 9 additions & 0 deletions pkg/cmd/run/shared/shared_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ func TestRunExportData(t *testing.T) {
},
output: `{"jobs":[{"completedAt":"2022-07-20T11:21:16Z","conclusion":"success","databaseId":123456,"name":"macos","startedAt":"2022-07-20T11:20:13Z","status":"completed","steps":[{"conclusion":"success","name":"Checkout","number":1,"status":"completed"}],"url":"https://example.com/OWNER/REPO/actions/runs/123456"},{"completedAt":"2022-07-20T11:23:16Z","conclusion":"error","databaseId":234567,"name":"windows","startedAt":"2022-07-20T11:20:55Z","status":"completed","steps":[{"conclusion":"error","name":"Checkout","number":2,"status":"completed"}],"url":"https://example.com/OWNER/REPO/actions/runs/234567"}]}`,
},
{
name: "exports workflow run with attempt count",
fields: []string{"attempt"},
run: Run{
Attempt: 1,
Jobs: []Job{},
},
output: `{"attempt":1}`,
},
}

for _, tt := range tests {
Expand Down