cli: use N/A as placeholder for old CREATED dates

[imjasonh]

Signed-off-by: Jason Hall jason@chainguard.dev

As suggested by @tianon in #3778 (comment)

ref #2995

@thaJeztah @tonistiigi

NB: This also fixes a test failure introduced in #3778, ignored because the test was skipped, because GitHub Runners don't run in UTC. The test that was skipped doesn't seem to actually require UTC though, so I've removed the skip and fixed the failure. The test now correctly checks that an image reported as being created 11 years ago prints 11 years ago.

edit: the UTC skip was added 3 years ago in #2427, I'm not sure if that's still needed for some situations. If we add it back we should make sure the test isn't skipped in CI at least.

- What I did

Changed placeholder text for old CREATED dates to N/A from an empty string

- How I did it

I carefully edited text files describing the behavior the program should follow.

- How to verify it

make test-unit, now with 100% more actually running tests.

- Description for the changelog

Images that report a created date before the years 2000 are displayed with a CREATED date of N/A.

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

[tianon]

❤️

[vdemeester]

🔥

[thaJeztah]

edit: the UTC skip was added 3 years ago in #2427, I'm not sure if that's still needed for some situations. If we add it back we should make sure the test isn't skipped in CI at least.

Thanks for tracing it back to that PR; I had to dig in memory what the failure was, but just gave it a try locally on this PR, and looks like we may still need "something";

=== RUN   TestHistoryContext_CreatedSince
    formatter_history_test.go:103: Expected "2009-11-10T23:00:00Z", was "2009-11-11T00:00:00+01:00"
--- FAIL: TestHistoryContext_CreatedSince (0.00s)

So looks like the issue is that the output is presented in local time, but the test is using a fixed value, assuming "local" time is UTC. I guess the best we can do for that is to instead of using a fixed string, to just format a string in the expected output format.

diff --git a/cli/command/image/formatter_history_test.go b/cli/command/image/formatter_history_test.go
index 66d75c257..66c5273bf 100644
--- a/cli/command/image/formatter_history_test.go
+++ b/cli/command/image/formatter_history_test.go
@@ -51,7 +51,9 @@ func TestHistoryContext_ID(t *testing.T) {
 }

 func TestHistoryContext_CreatedSince(t *testing.T) {
-       dateStr := "2009-11-10T23:00:00Z"
+       longerAgo := time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC)
+       dateStr := longerAgo.Local().Format(time.RFC3339)
+
        var ctx historyContext
        cases := []historyCase{
                {
@@ -63,7 +65,7 @@ func TestHistoryContext_CreatedSince(t *testing.T) {
                },
                {
                        historyContext{
-                               h:     image.HistoryResponseItem{Created: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC).Unix()},
+                               h:     image.HistoryResponseItem{Created: longerAgo.Unix()},
                                trunc: false,
                                human: false,
                        }, dateStr, ctx.CreatedSince,

[imjasonh]

Done, thanks for that suggestion.

FWIW there's another test in this area, TestNewHistoryCommandSuccess, that's skipped when not in UTC, and that seems to fail when that skip is removed:

=== FAIL: cli/command/image TestNewHistoryCommandSuccess (0.01s)
    history_test.go:101: assertion failed: 
        --- expected
        +++ actual
        @@ -1,3 +1,3 @@
        -IMAGE···············CREATED··················CREATED·BY··········SIZE················COMMENT
        -123456789012········Less·than·a·second·ago·······················0B··················
        +IMAGE··········CREATED··················CREATED·BY···SIZE······COMMENT
        +123456789012···Less·than·a·second·ago················0B········

Wrapping the table-driven test in a t.Run so the golden.Assert doesn't stop the test, there are actually two failures, both seemingly due to spacing:

=== FAIL: cli/command/image TestNewHistoryCommandSuccess/simple (0.00s)
    history_test.go:102: assertion failed: 
        --- expected
        +++ actual
        @@ -1,3 +1,3 @@
        -IMAGE···············CREATED··················CREATED·BY··········SIZE················COMMENT
        -123456789012········Less·than·a·second·ago·······················0B··················
        +IMAGE··········CREATED··················CREATED·BY···SIZE······COMMENT
        +123456789012···Less·than·a·second·ago················0B········
         
        
        
        You can run 'go test . -test.update-golden' to automatically update testdata/history-command-success.simple.golden to the new expected value.'
        
    --- FAIL: TestNewHistoryCommandSuccess/simple (0.00s)

=== FAIL: cli/command/image TestNewHistoryCommandSuccess/non-human (0.00s)
    history_test.go:102: assertion failed: 
        --- expected
        +++ actual
        @@ -1,3 +1,3 @@
        -IMAGE···············CREATED·AT·············CREATED·BY··········SIZE················COMMENT
        -abcdef··············2017-01-01T12:00:03Z···rose················0···················new·history·item!
        +IMAGE·····CREATED·AT·············CREATED·BY···SIZE······COMMENT
        +abcdef····2017-01-01T12:00:03Z···rose·········0·········new·history·item!
         
        
        
        You can run 'go test . -test.update-golden' to automatically update testdata/history-command-success.non-human.golden to the new expected value.'
        
    --- FAIL: TestNewHistoryCommandSuccess/non-human (0.00s)

=== FAIL: cli/command/image TestNewHistoryCommandSuccess (0.01s)

I can fix these tests as well, but I'd prefer to do it in a followup PR since they're unrelated to this change.

edit: #3781

[codecov-commenter]

Codecov Report

Merging #3779 (c864b01) into master (06cbd26) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head c864b01 differs from pull request most recent head 90db6b8. Consider uploading reports for the commit 90db6b8 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3779      +/-   ##
==========================================
+ Coverage   59.26%   59.28%   +0.02%     
==========================================
  Files         288      288              
  Lines       24632    24632              
==========================================
+ Hits        14597    14602       +5     
+ Misses       9164     9160       -4     
+ Partials      871      870       -1     

[thaJeztah]

Thanks for updating this one! Could you squash the two commits? LGTM other than that 👍

[imjasonh]

Thanks for updating this one! Could you squash the two commits? LGTM other than that 👍

Should be ready to go. Let me know if there's anything else you'd like.

(And thanks for the reviews and approvals!)

[thaJeztah]

LGTM

let's get this one in; thanks!!