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 for panic when submitting non-existent version for job history CLI command #4577

Merged
merged 2 commits into from
Aug 14, 2018
Merged

Fix for panic when submitting non-existent version for job history CLI command #4577

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35f3973
fix panic for job history cli command when used with non-existent job…
chelseakomlo Aug 10, 2018
1ec0852
spelling fix
chelseakomlo Aug 14, 2018
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
4 changes: 4 additions & 0 deletions command/job_history.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ func (c *JobHistoryCommand) formatJobVersions(versions []*api.Job, diffs []*api.
}

func (c *JobHistoryCommand) formatJobVersion(job *api.Job, diff *api.JobDiff, nextVersion uint64, full bool) error {
if job == nil {
return fmt.Errorf("Error printing job history for non-existing job or job version")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a test case for this in job_history_test.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are not any success case tests for job_history or similar commands such as job_inspect or job_revert- I tried a couple different test setups but wasn't able to get a successful test passing for this. I'm going to merge as is but we can look at what a success test case could look like.

basic := []string{
fmt.Sprintf("Version|%d", *job.Version),
fmt.Sprintf("Stable|%v", *job.Stable),
Expand Down