cmd: Add ref-id auto-discovery to resource upgrade #92
Conversation
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got, err := UpgradeStateless(tt.args.params) | ||
| if !reflect.DeepEqual(err, tt.err) { | ||
| if tt.err != nil && err.Error() != tt.err.Error() { |
There was a problem hiding this comment.
I think we should settle on which error assertions we want to follow. It's not a big deal, but we're not asserting the same thing, an error could be returned by the function anyway and we wouldn't catch it unless there's something in the err field of the test structure, which won't be if the test structure is not asserting for an error different than nil.
There was a problem hiding this comment.
I prefer using !reflect.DeepEqual(err, tt.err) as well :) In this case, unfortunately, when I added the last test case I started getting the following error, so I had to change the assertion.
--- FAIL: TestUpgradeStateless/succeeds_when_RefID_is_not_set (0.00s)
/Users/karen/go/src/github.com/elastic/ecctl/pkg/deployment/depresource/upgrade_stateless_test.go:172: UpgradeStateless() error = , wantErr <nil>
There was a problem hiding this comment.
DeepEquals probably fails because the tests are of different type...
|
Updated the PR to use |
Description
Adds RefID auto-discovery when the
--ref-idflag is not set.Related Issues
Closes: #91
Motivation and Context
Fixes a bug where this value was not being auto-discovered as per the flag's description.
--ref-id string Optional deployment RefId, if not set, the RefId will be auto-discoveredHow Has This Been Tested?
Manually and with unit tests
Types of Changes
Readiness Checklist