Skip to content

Conversation

@thaJeztah
Copy link
Member

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)

…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
Copy link
Member

@rumpl rumpl Jan 22, 2024

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?

Copy link
Member Author

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 😂

Copy link
Member

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

Copy link
Member Author

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).

@thaJeztah thaJeztah merged commit 42a0788 into moby:master Jan 22, 2024
@thaJeztah thaJeztah deleted the integration_cli_api_versions branch January 22, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants