Fix ImageCollectionTest.test_pull_multiple flakiness #2485
Conversation
|
ping @ulyssessouza @shin- ptal |
thaJeztah
force-pushed
the
fix_ImageCollectionTest_test_pull_multiple
branch
from
71c20ba
to
9f65c2b
Compare
|
Ah booh. It's failing. Not sure what I did wrong 🤔 |
| assert len(images) >= 1 | ||
| found = False | ||
| for img in images: | ||
| if 'hello-world:latest' == img.attrs['RepoTags']: |
There was a problem hiding this comment.
Hi @thaJeztah
Could you try to use Unicode for image name, like u'hello-world:latest'?
There was a problem hiding this comment.
Sure, I can try that (would that be a bit odd though, given that image references only support a-z0-9 ? 🤔 or does python return it as unicode? (I'm not really a Python expert 😅)
There was a problem hiding this comment.
Python 2 doesn't use Unicode as default.
Also, I forgot that img.attrs['RepoTags'] is a list, so you can change the comparator == to in and the test will pass.
There was a problem hiding this comment.
Doh! You're right, completely forgot about that; yes, that should be the problem
There was a problem hiding this comment.
Yeah, Unicode wasn't the bad guy this time. hahaha
thaJeztah
force-pushed
the
fix_ImageCollectionTest_test_pull_multiple
branch
2 times, most recently
from
b7c4d7a
to
39c1fbb
Compare
|
all green now! thanks for your help (and review) @feliperuhland 🤗 |
| assert 'hello-world:latest' in images[0].attrs['RepoTags'] | ||
| assert len(images) >= 1 | ||
| found = False | ||
| for img in images: |
There was a problem hiding this comment.
Could you use any() instead of doing a for loop. E.g. something like assert any('hello-world:latest' in img[0].attrs['RepoTags'] for img in images)?
There was a problem hiding this comment.
Ah, yes, sure. Didn't know of that approach 🤗
There was a problem hiding this comment.
It's just img.attrs, not img[0].attrs.
Edit: the code looks better.
thaJeztah
force-pushed
the
fix_ImageCollectionTest_test_pull_multiple
branch
from
39c1fbb
to
9d5d963
Compare
The ImageCollectionTest.test_pull_multiple test performs a `docker pull` without
a `:tag` specified) to pull all tags of the given repository (image).
After pulling the image, the image(s) pulled are checked to verify if the list
of images contains the `:latest` tag.
However, the test assumes that all tags of the image are tags for the same
version of the image (same digest), and thus a *single* image is returned, which
is not always the case.
Currently, the `hello-world:latest` and `hello-world:linux` tags point to a
different digest, therefore the `client.images.pull()` returns multiple images:
one image for digest, making the test fail:
=================================== FAILURES ===================================
____________________ ImageCollectionTest.test_pull_multiple ____________________
tests/integration/models_images_test.py:90: in test_pull_multiple
assert len(images) == 1
E AssertionError: assert 2 == 1
E + where 2 = len([<Image: 'hello-world:linux'>, <Image: 'hello-world:latest'>])
This patch updates the test to not assume a single image is returned, and instead
loop through the list of images and check if any of the images contains the
`:latest` tag.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
fix_ImageCollectionTest_test_pull_multiple
branch
from
9d5d963
to
940805d
Compare
|
@zappy-shu @ulyssessouza @feliperuhland updated; ptal |
|
@thaJeztah looks great |
relates to moby/moby#40340 (moby/moby#40340 (comment))
relates to moby/moby#40343
introduced in #1883
The ImageCollectionTest.test_pull_multiple test performs a
docker pullwithouta
:tagspecified) to pull all tags of the given repository (image).After pulling the image, the image(s) pulled are checked to verify if the list
of images contains the
:latesttag.However, the test assumes that all tags of the image are tags for the same
version of the image (same digest), and thus a single image is returned, which
is not always the case.
Currently, the
hello-world:latestandhello-world:linuxtags point to adifferent digest, therefore the
client.images.pull()returns multiple images:one image for digest, making the test fail:
This patch updates the test to not assume a single image is returned, and instead
loop through the list of images and check if any of the images contains the
:latesttag.