-
Notifications
You must be signed in to change notification settings - Fork 46
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
test: Improved device unit test cases and fixed issue #340
Conversation
89e6766
to
b04a49d
Compare
test/helper/helper.go
Outdated
@@ -74,6 +74,29 @@ func IsDeviceStateActive(deviceId string) (bool, error) { | |||
return false, err | |||
} | |||
|
|||
func IsDeviceStateInActive(deviceId string) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than create a separate function, the existing helper could be updated to accept a state
as an argument; that way we can avoid creating a new function if we find a third or fourth state we need to wait for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
test/helper/helper.go
Outdated
// Sleep for the specified interval | ||
time.Sleep(retryInterval) | ||
} | ||
return false, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is going to return a nil
error; instead we should return an error that describes what happened. #339 shows how we could do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
b04a49d
to
9e69ca3
Compare
Was looking at this, seems like the state shouldn't be a string but instead some type defined in the generated API files. |
@@ -125,8 +125,13 @@ func (c *Client) Create() *cobra.Command { | |||
if billingCycle != "" { | |||
facilityDeviceRequest.DeviceCreateInFacilityInput.SetBillingCycle(*validBillingCycle) | |||
} | |||
if alwaysPXE { | |||
facilityDeviceRequest.DeviceCreateInFacilityInput.SetAlwaysPxe(alwaysPXE) | |||
if alwaysPXE != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is anything left in this PR that hasn't been addressed in another PR? Anything that this PR aimed to do that it hasn't done?
Should we close this?
Closing this since everything in here appears to have been addressed in other PRs. |
Description:
We improved the device unit cases
Issue fixed?
#257