Skip to content

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Nov 29, 2023

- What I did
Skip part of TestListDanglingImagesWithDigests which tests graphdriver implementation specific behavior of docker images --filter dangling=true.

- How I did it

- How to verify it
Check TestListDanglingImagesWithDigests test result

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@@ -341,6 +341,13 @@ func (s *DockerRegistrySuite) TestListDanglingImagesWithDigests(c *testing.T) {
// make sure repo shown, tag=<none>, digest = $digest1
re1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1.String() + `\s`)
Copy link
Member

Choose a reason for hiding this comment

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

Is this case already covered by TestListImagesWithDigests above?

If it is, it may be cleaner to t.Skip() the whole test

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this one test is actually multiple sub-tests 🤔 we could make sub-tests and skip those, but I guess we'd have to (temporarily) disable the "integration-cli is deprecated" check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, looks the same.
Let's just skip the whole test.

@vvoland vvoland force-pushed the c8d-skip-TestListDanglingImagesWithDigests branch from 96797e1 to 308825c Compare November 30, 2023 12:57
@@ -341,6 +344,7 @@ func (s *DockerRegistrySuite) TestListDanglingImagesWithDigests(c *testing.T) {
// make sure repo shown, tag=<none>, digest = $digest1
re1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1.String() + `\s`)
assert.Assert(c, re1.MatchString(out), "expected %q: %s", re1.String(), out)

Copy link
Member

Choose a reason for hiding this comment

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

oh! unrelated change (if you still have the branch open locally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, sorry! 😄

Copy link
Member

Choose a reason for hiding this comment

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

No worries! Thought you still had the branch at hand, so would be easy to fix (and wouldn't have been a massive disaster if it was left)

Skip TestListDanglingImagesWithDigests which tests graphdriver
implementation specific behavior of `docker images --filter
dangling=true`.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the c8d-skip-TestListDanglingImagesWithDigests branch from 308825c to fcb89da Compare November 30, 2023 13:01
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 08035dc into moby:master Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing containerd-integration Issues and PRs related to containerd integration status/2-code-review
Projects
Development

Successfully merging this pull request may close these issues.

3 participants