-
Notifications
You must be signed in to change notification settings - Fork 18.9k
integration-cli: adjust inspect tests to use current API version #47154
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
integration-cli: adjust inspect tests to use current API version #47154
Conversation
…version This test was added in f301c57 to test inspect output for API > v1.21, however, it was pinned to API v1.21, which is now deprecated. Remove the fixed version, as the intent was to test "current" API versions (API v1.21 and up), Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was added in f301c57 to test inspect output for API > v1.21, however, it was pinned to API v1.21, which is now deprecated. Remove the fixed version, as the intent was to test "current" API versions (API v1.21 and up), Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
| assert.Assert(c, len(settings.IPAddress) != 0) | ||
| } | ||
|
|
||
| // Inspect for API v1.21 and up; see |
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.
Does this comment make any sense at all now that we don’t do anything soecial with the api version?
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.
Honestly, I didn't want to go through the "disable integration-cli check -> merge -> re-enable" cycle to rename the test (removing 121), and thought I'd leave some crumble-patch to why it was named like this 😂
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.
Oh, was on my phone, didn't see the test name
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.
No worries; it's a good question, and perhaps it's "too much info". I also anticipated that perhaps we'd have version-dependent changes again in future (but likely we should rewrite / move this test to integration then).
integration-cli: TestInspectAPIBridgeNetworkSettings121: use current version
This test was added in f301c57 to test
inspect output for API > v1.21, however, it was pinned to API v1.21,
which is now deprecated.
Remove the fixed version, as the intent was to test "current" API versions
(API v1.21 and up),
integration-cli: TestInspectAPIMultipleNetworks: use current version
This test was added in f301c57 to test
inspect output for API > v1.21, however, it was pinned to API v1.21,
which is now deprecated.
Remove the fixed version, as the intent was to test "current" API versions
(API v1.21 and up),
- A picture of a cute animal (not mandatory but encouraged)