Test TestClear_WithRevoke_ActiveToken makes a buggy comparison

Pretty sure this isn't reported. I looked and didn't see any similar matches.

This is agentically generated, so caveat emptor, but the logic passes my (admittedly poor) ability to sniff-test.

Summary

The issue:

This is found in internal/commands/cluster/agent/token_cache/clear/clear.go:300-303

  • The test TestClear_WithRevoke_ActiveToken creates a token that expires 24 hours from now
  • gitlab.ISOTime only stores the date component (midnight UTC of that date)
  • After JSON serialization/deserialization, the expiration becomes YYYY-MM-DD 00:00:00 UTC
  • The test fails if it runs after midnight UTC on that date, even though the token shouldn't expire until the end of that day

The problem is time.Time(*token.Token.ExpiresAt).Before(time.Now().UTC()) compares full datetime against full datetime, but ExpiresAt only has a date component (midnight UTC), so it becomes expired the moment 1 second passes midnight on that date.

The proposed fix is to compare dates only, not datetimes.

I'm not sure if that's the right call, honestly. but I figure passing the info along is helpful.

Steps to reproduce

make tests at different times of day? or with a different reference time? not sure why my env is breaking on this an y'alls isn't..

What is the current bug behavior?

tests fail in my env

What is the expected correct behavior?

tests not fail in my env? 😄

Relevant logs and/or screenshots 

=== Failed
=== FAIL: internal/commands/cluster/agent/token_cache/clear TestClear_WithRevoke_ActiveToken (0.00s)
    clear_command_test.go:126:
        	Error Trace:	/tmp/cli/internal/commands/cluster/agent/token_cache/clear/clear_command_test.go:126
        	Error:      	"Found 1 cached token(s) to clear.\nRevoking tokens on GitLab server...\nToken for agent 10 is expired, skipping revocation.\nClearing tokens from cache...\nCleared token for agent 10 from filesystem.\nSuccessfully cleared 1 token(s) from cache.\n" does not contain "Revoking token for agent 10"
        	Test:       	TestClear_WithRevoke_ActiveToken
    clear_command_test.go:127:
        	Error Trace:	/tmp/cli/internal/commands/cluster/agent/token_cache/clear/clear_command_test.go:127
        	Error:      	"Found 1 cached token(s) to clear.\nRevoking tokens on GitLab server...\nToken for agent 10 is expired, skipping revocation.\nClearing tokens from cache...\nCleared token for agent 10 from filesystem.\nSuccessfully cleared 1 token(s) from cache.\n" does not contain "Successfully revoked token for agent 10"
        	Test:       	TestClear_WithRevoke_ActiveToken
    controller.go:97: missing call(s) to *testing.MockPersonalAccessTokensServiceInterface.RevokePersonalAccessTokenByID(is equal to 456 (int64), is anything) /tmp/cli/internal/commands/cluster/agent/token_cache/clear/clear_command_test.go:119
    controller.go:97: aborting test due to missing call(s)

Possible fixes

internal/commands/cluster/agent/token_cache/clear/clear.go:300-303

The root cause in clear.go:300-303:

if token.Token.ExpiresAt != nil && time.Time(*token.Token.ExpiresAt).Before(time.Now().UTC()) {
    fmt.Fprintf(o.io.StdOut, "Token for agent %d is expired, skipping revocation.\n", token.AgentID)
    continue
}

This compares midnight UTC vs now UTC instead of comparing just the dates.

Fix: Compare date components only:

if token.Token.ExpiresAt != nil {
    expiresAt := time.Time(*token.Token.ExpiresAt)
    now := time.Now().UTC()
    isExpired := expiresAt.Year() < now.Year() ||
        (expiresAt.Year() == now.Year() && expiresAt.Month() < now.Month()) ||
        (expiresAt.Year() == now.Year() && expiresAt.Month() == now.Month() && expiresAt.Day() < now.Day())
    if isExpired {
        fmt.Fprintf(o.io.StdOut, "Token for agent %d is expired, skipping revocation.\n", token.AgentID)
        continue
    }
}
Edited by Wolf Noble