-
Notifications
You must be signed in to change notification settings - Fork 176
Conversation
Added a created date field to the custom payload of the bundle.json. This is packaged as part of the app image at build time. The CREATED column has been added to the `app image ls` output. This displays the time since the app image was built in the same human readable format as `image ls`. As the creation date is dynamic the app image ID will be unique for each build even if nothing else has changed. To adjust the tests for this the golden bundle.json is now checked using regex matching instead of string matching. Simalarly, the e2e tests for `app image ls` are checked with regexes for the CREATED column as the exact value will depend on how long the tests take to run. Signed-off-by: Nick Adcock <nick.adcock@docker.com>
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #735 +/- ##
==========================================
+ Coverage 69.3% 70.03% +0.73%
==========================================
Files 63 64 +1
Lines 3388 3651 +263
==========================================
+ Hits 2348 2557 +209
- Misses 731 747 +16
- Partials 309 347 +38
Continue to review full report at Codecov.
|
@@ -23,7 +24,11 @@ func insertBundles(t *testing.T, cmd icmd.Cmd) { | |||
|
|||
func assertImageListOutput(t *testing.T, cmd icmd.Cmd, expected string) { | |||
result := icmd.RunCmd(cmd).Assert(t, icmd.Success) | |||
match, _ := regexp.MatchString(expected, result.Stdout()) | |||
stdout := result.Stdout() | |||
match, _ := regexp.MatchString(expected, stdout) |
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.
I guess the error could be asserted here.
match, _ := regexp.MatchString(expected, stdout) | ||
if !match { | ||
fmt.Println(stdout) | ||
} | ||
assert.Assert(t, match) |
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.
I think you can add a third argument here, instead of checking match
before assertion.
assert.Assert(t, match, stdout)
@@ -46,6 +51,7 @@ func verifyImageIDListOutput(t *testing.T, cmd icmd.Cmd, count int, distinct int | |||
for scanner.Scan() { | |||
lines = append(lines, scanner.Text()) | |||
counter[scanner.Text()]++ | |||
fmt.Println(scanner.Text()) |
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.
Should we really print here?
if createdPayload, ok := payload.(packager.CustomPayloadCreated); ok { | ||
return units.HumanDuration(time.Now().UTC().Sub(createdPayload.CreatedTime())) + " ago" | ||
} | ||
return "" |
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.
I wonder if it should be empty ""
or "N/A"
? @thaJeztah WDYT?
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.
Ideally, would use a go template, which would allow setting N/A
in the template (separating presentation from the value); see docker/cli#2107
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.
Yep right, we're fixing it in a follow-up, thanks @thaJeztah 😸
- What I did
Added a created date field to the custom payload of the bundle.json which is used to display the time since the app image was built in human readable format on
docker app image ls
as theCREATED
column.- How I did it
Created a v1.0 struct for the custom payload in bundle.json that stores the creation date at build time. A constant is used to track which version of the custom payload is to be deserialized rather than using the
docker app version
. This is to make it easier to check which struct matches which versions.As the entire bundle.json is deserialized at once the custom payload needs to be re-serialized before deserializing into the concrete type as it is stored as a simple
map[string]interface{}
.The human readable output is done using the
github.com/docker/go-units
package, same as withdocker image ls
.- How to verify it
Run a
docker app build
. The resulting bundle.json should have a custom section similar to the following:Note that the
created
field should be in UTC.Running
docker app image ls
should print aCREATED
column showing the amount of time since the app image was built or blank if the image was built using an older version ofdocker app
.Example output:
- Description for the changelog
Added the creation date to the App bundle and prints the time since creation in the
docker app image ls
output- A picture of a cute animal (not mandatory)