Skip to content

Commit

Permalink
Fix time comparison issues with buildscripts due to inconsistent time…
Browse files Browse the repository at this point in the history
… formatting
  • Loading branch information
Naatan committed Nov 8, 2024
1 parent c20a2fb commit 543089c
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 23 deletions.
2 changes: 1 addition & 1 deletion internal/runbits/buildscript/buildscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

const testProject = "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000"
const testTime = "2000-01-01T00:00:00.000Z"
const testTime = "2000-01-01T00:00:00Z"

func checkoutInfo(project, time string) string {
return "```\n" +
Expand Down
12 changes: 11 additions & 1 deletion pkg/buildscript/buildscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/ActiveState/cli/internal/condition"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/rtutils/ptr"
"github.com/ActiveState/cli/internal/sliceutils"
"github.com/brunoga/deep"
Expand Down Expand Up @@ -65,7 +66,15 @@ func (b *BuildScript) SetAtTime(t time.Time, override bool) {
if b.atTime != nil && !override {
return
}
b.atTime = &t
// Ensure time is RFC3339 formatted, even though it's not actually stored at that format, it does still
// affect the specificity of the data stored and can ultimately lead to inconsistencies if not explicitly handled.
t2, err := time.Parse(time.RFC3339, t.Format(time.RFC3339))
if err != nil {
// Pointless error check as this should never happen, but you know what they say about assumptions
logging.Error("Error parsing time: %s", err)
}
b.atTime = ptr.To(t2)
_ = b.atTime
}

func (b *BuildScript) Equals(other *BuildScript) (bool, error) {
Expand All @@ -90,6 +99,7 @@ func (b *BuildScript) Equals(other *BuildScript) (bool, error) {
if err != nil {
return false, errs.Wrap(err, "Unable to marshal other buildscript")
}

return string(myBytes) == string(otherBytes), nil
}

Expand Down
13 changes: 6 additions & 7 deletions pkg/buildscript/buildscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/ActiveState/cli/internal/environment"
"github.com/ActiveState/cli/internal/fileutils"
"github.com/go-openapi/strfmt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -118,7 +117,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {
// Ensure that legacy at_time is preserved in the buildscript.
atTime := script.AtTime()
require.NotNil(t, atTime)
require.Equal(t, initialTimeStamp, atTime.Format(strfmt.RFC3339Millis))
require.Equal(t, initialTimeStamp, atTime.Format(time.RFC3339))

data, err = script.MarshalBuildExpression()
require.NoError(t, err)
Expand All @@ -128,12 +127,12 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {
assert.NotContains(t, string(data), initialTimeStamp)

// Update the time in the build script but don't override the existing time
updatedTime, err := time.Parse(strfmt.RFC3339Millis, updatedTimeStamp)
updatedTime, err := time.Parse(time.RFC3339, updatedTimeStamp)
require.NoError(t, err)
script.SetAtTime(updatedTime, false)

// The updated time should be reflected in the build script
require.Equal(t, initialTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis))
require.Equal(t, initialTimeStamp, script.AtTime().Format(time.RFC3339))

data, err = script.Marshal()
require.NoError(t, err)
Expand All @@ -145,7 +144,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {

// Now override the time in the build script
script.SetAtTime(updatedTime, true)
require.Equal(t, updatedTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis))
require.Equal(t, updatedTimeStamp, script.AtTime().Format(time.RFC3339))

data, err = script.Marshal()
require.NoError(t, err)
Expand All @@ -165,7 +164,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {
// TestExpressionToScript tests that creating a build script from a given Platform build expression
// and at time produces the expected result.
func TestExpressionToScript(t *testing.T) {
ts, err := time.Parse(strfmt.RFC3339Millis, testTime)
ts, err := time.Parse(time.RFC3339, testTime)
require.NoError(t, err)

script := New()
Expand Down Expand Up @@ -193,7 +192,7 @@ func TestScriptToExpression(t *testing.T) {

func TestOutdatedScript(t *testing.T) {
_, err := Unmarshal([]byte(
`at_time = "2000-01-01T00:00:00.000Z"
`at_time = "2000-01-01T00:00:00Z"
main = runtime
`))
assert.Error(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/buildscript/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"fmt"
"strconv"
"strings"
"time"

"github.com/go-openapi/strfmt"
"github.com/thoas/go-funk"
)

Expand All @@ -31,7 +31,7 @@ func (b *BuildScript) Marshal() ([]byte, error) {
buf.WriteString("```\n")
buf.WriteString("Project: " + b.project + "\n")
if b.atTime != nil {
buf.WriteString("Time: " + b.atTime.Format(strfmt.RFC3339Millis) + "\n")
buf.WriteString("Time: " + b.atTime.Format(time.RFC3339) + "\n")
}
buf.WriteString("```\n\n")

Expand Down
4 changes: 2 additions & 2 deletions pkg/buildscript/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/stretchr/testify/require"
)

const mergeATime = "2000-01-01T00:00:00.000Z"
const mergeBTime = "2000-01-02T00:00:00.000Z"
const mergeATime = "2000-01-01T00:00:00Z"
const mergeBTime = "2000-01-02T00:00:00Z"

func TestMergeAdd(t *testing.T) {
scriptA, err := Unmarshal([]byte(
Expand Down
6 changes: 3 additions & 3 deletions pkg/buildscript/raw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

const testProject = "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000"
const testTime = "2000-01-01T00:00:00.000Z"
const testTime = "2000-01-01T00:00:00Z"

func checkoutInfoString(project, time string) string {
return "```\n" +
Expand Down Expand Up @@ -43,7 +43,7 @@ main = runtime
`))
require.NoError(t, err)

atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00.000Z")
atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00Z")
require.NoError(t, err)
atTime := time.Time(atTimeStrfmt)

Expand Down Expand Up @@ -121,7 +121,7 @@ main = merge(
`))
require.NoError(t, err)

atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00.000Z")
atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00Z")
require.NoError(t, err)
atTime := time.Time(atTimeStrfmt)

Expand Down
12 changes: 8 additions & 4 deletions pkg/buildscript/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/rtutils/ptr"
)

const atTimeKey = "at_time"
Expand Down Expand Up @@ -72,11 +71,16 @@ func Unmarshal(data []byte) (*BuildScript, error) {

project = info.Project

atTimeStr, err := strfmt.ParseDateTime(info.Time)
atTimeVal, err := time.Parse(time.RFC3339, info.Time)
if err != nil {
return nil, errs.Wrap(err, "Invalid timestamp: %s", info.Time)
// Older buildscripts used microsecond specificity
atDateTime, err := strfmt.ParseDateTime(info.Time)
if err != nil {
return nil, errs.Wrap(err, "Invalid timestamp: %s", info.Time)
}
atTimeVal = time.Time(atDateTime)
}
atTime = ptr.To(time.Time(atTimeStr))
atTime = &atTimeVal
}

return &BuildScript{raw, project, atTime}, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/buildscript/unmarshal_buildexpression.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error {
atTimeNode.Str = nil
atTimeNode.Ident = ptr.To("TIME")
// Preserve the original at_time found in the solve node.
b.atTime = ptr.To(time.Time(atTime))
b.SetAtTime(time.Time(atTime), true)
} else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" {
atTimeNode.Ident = ptr.To("TIME")
}
Expand Down
3 changes: 1 addition & 2 deletions scripts/to-buildscript/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/fileutils"
"github.com/ActiveState/cli/pkg/buildscript"
"github.com/go-openapi/strfmt"
)

func main() {
Expand Down Expand Up @@ -45,7 +44,7 @@ func main() {
project := "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000"
var atTime *time.Time
if len(os.Args) == (2 + argOffset) {
t, err := time.Parse(strfmt.RFC3339Millis, os.Args[1+argOffset])
t, err := time.Parse(time.RFC3339, os.Args[1+argOffset])
if err != nil {
panic(errs.JoinMessage(err))
}
Expand Down

0 comments on commit 543089c

Please sign in to comment.