c8d/integration: Adjust TestSaveCheckTimes#46848
Conversation
The graphdriver implementation sets the ModTime of all image content to match the `Created` time from the image config, whereas the containerd's archive export code just leaves it empty (zero). Adjust the test in the case where containerd integration is enabled to check if config file ModTime is equal to zero (UNIX epoch) instead. This behaviour is not a part of the Docker Image Specification and the intention behind introducing it was to make the `docker save` produce the same archive regardless of the time it was performed. It would also be a bit problematic with the OCI archive layout which can contain multiple images referencing the same content. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
09e0d14 to
05523e2
Compare
| assert.Check(t, is.Equal(info.ModTime(), time.Unix(0, 0))) | ||
| } else { | ||
| assert.Check(t, is.Equal(info.ModTime().Format(time.RFC3339), created.Format(time.RFC3339))) |
There was a problem hiding this comment.
Perhaps not an issue, but I recall there were potentially issues with comparing timestamps, so I'd blame myself for not commenting otherwise; should these be using info.ModTime().IsZero() and info.ModTime().Equal(...) ?
There was a problem hiding this comment.
info.ModTime().IsZero() is not the same as == time.Unix(0, 0) (0000-00-00 != 1970-01-01).
As for using time.Equal - in general it should be used instead of plain == but I think it's enough for this specific test and it provides a meaningful assert message for us. Worst case we'll get a false negative but that will be easily detectable.
There was a problem hiding this comment.
info.ModTime().IsZero() is not the same as == time.Unix(0, 0) (0000-00-00 != 1970-01-01).
Oh! Yes, you're right. Wrote that up without thinking.
I think it's enough for this specific test and it provides a meaningful assert message for us.
Yeah, makes sense; that's a good point. Was considering that when writing (whether "correct" vs "usable diff" was worth the trade-off)
The graphdriver implementation sets the ModTime of all image content to match the
Createdtime from the image config, whereas the containerd's archive export code just leaves it empty (zero).Adjust the test in the case where containerd integration is enabled to check if config file ModTime is equal to zero (UNIX epoch) instead.
This behaviour is not a part of the Docker Image Specification and the intention behind introducing it was to make the
docker saveproduce the same archive regardless of the time it was performed (see #16076 and #8819 (comment)).It would also be a bit problematic with the OCI archive layout which can contain multiple images referencing the same content.
- What I did
- How I did it
- How to verify it
Watch TestSaveCheckTimes in CI.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)