Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix merge integration test #3552

Merged
merged 8 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/runners/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func (i *Install) renderUserFacing(reqs requirements) {
}

func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time) error {
script.SetAtTime(ts)
script.SetAtTime(ts, true)
for _, req := range requirements {
requirement := types.Requirement{
Namespace: req.Resolved.Namespace,
Expand Down
2 changes: 1 addition & 1 deletion internal/runners/platforms/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (a *Add) Run(params AddRunParams) (rerr error) {

// Prepare updated buildscript
script := oldCommit.BuildScript()
script.SetAtTime(ts)
script.SetAtTime(ts, true)
script.AddPlatform(*platform.PlatformID)

// Update local checkout and source runtime changes
Expand Down
2 changes: 1 addition & 1 deletion internal/runners/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (u *Upgrade) Run(params *Params) (rerr error) {
if err != nil {
return errs.Wrap(err, "Failed to fetch latest timestamp")
}
bumpedBS.SetAtTime(ts)
bumpedBS.SetAtTime(ts, true)

// Since our platform is commit based we need to create a commit for the "after" buildplan, even though we may not
// end up using it it the user doesn't confirm the upgrade.
Expand Down
8 changes: 7 additions & 1 deletion pkg/buildscript/buildscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ func (b *BuildScript) AtTime() *time.Time {
return b.atTime
}

func (b *BuildScript) SetAtTime(t time.Time) {
// SetAtTime sets the AtTime value, if the buildscript already has an AtTime value
// and `override=false` then the value passed here will be discarded.
// Override should in most cases only be true if we are making changes to the buildscript.
func (b *BuildScript) SetAtTime(t time.Time, override bool) {
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
if b.atTime != nil && !override {
return
}
b.atTime = &t
}

Expand Down
70 changes: 69 additions & 1 deletion pkg/buildscript/buildscript_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package buildscript

import (
"fmt"
"path/filepath"
"testing"
"time"

"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 @@ -94,6 +98,70 @@ func TestRoundTripFromBuildExpression(t *testing.T) {
require.Equal(t, string(basicBuildExpression), string(data))
}

func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {
wd, err := environment.GetRootPath()
require.NoError(t, err)

initialTimeStamp := "2024-10-15T16:37:06.260Z"
updatedTimeStamp := "2024-10-15T16:37:06.261Z"

data, err := fileutils.ReadFile(filepath.Join(wd, "pkg", "buildscript", "testdata", "buildexpression-roundtrip-legacy.json"))
require.NoError(t, err)

// The initial build expression does not use the new at_time format
assert.NotContains(t, string(data), "$at_time")
assert.Contains(t, string(data), initialTimeStamp)

script := New()
require.NoError(t, script.UnmarshalBuildExpression(data))

// 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))

data, err = script.MarshalBuildExpression()
require.NoError(t, err)

// When the build expression is unmarshalled it should now use the new at_time format
assert.Contains(t, string(data), "$at_time")
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)
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))

data, err = script.Marshal()
require.NoError(t, err)

// The marshalled build script should NOT contain the updated time
// in the Time block at the top of the script.
assert.Contains(t, string(data), initialTimeStamp)
assert.NotContains(t, string(data), fmt.Sprintf("Time: %s", updatedTime))

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

data, err = script.Marshal()
require.NoError(t, err)

// The marshalled build script should NOW contain the updated time
// in the Time block at the top of the script.
assert.Contains(t, string(data), updatedTimeStamp)
assert.NotContains(t, string(data), fmt.Sprintf("Time: %s", initialTimeStamp))

data, err = script.MarshalBuildExpression()
require.NoError(t, err)

// The build expression representation should now use the new at_time format
assert.Contains(t, string(data), "$at_time")
}

// 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) {
Expand All @@ -102,7 +170,7 @@ func TestExpressionToScript(t *testing.T) {

script := New()
script.SetProject(testProject)
script.SetAtTime(ts)
script.SetAtTime(ts, false)
require.NoError(t, script.UnmarshalBuildExpression(basicBuildExpression))

data, err := script.Marshal()
Expand Down
2 changes: 1 addition & 1 deletion pkg/buildscript/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (b *BuildScript) Merge(other *BuildScript, strategies *mono_models.MergeStr
// When merging build scripts we want to use the most recent timestamp
atTime := other.AtTime()
if atTime != nil && b.AtTime() != nil && atTime.After(*b.AtTime()) {
b.SetAtTime(*atTime)
b.SetAtTime(*atTime, true)
}

return nil
Expand Down
43 changes: 43 additions & 0 deletions pkg/buildscript/testdata/buildexpression-roundtrip-legacy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"let": {
"sources": {
"solve": {
"at_time": "2024-10-15T16:37:06.260Z",
"solver_version": null,
"platforms": [
"46a5b48f-226a-4696-9746-ba4d50d661c2",
"78977bc8-0f32-519d-80f3-9043f059398c",
"7c998ec2-7491-4e75-be4d-8885800ef5f2"
],
"requirements": [
{
"name": "community_artifact",
"namespace": "internal"
},
{
"name": "python",
"namespace": "language",
"version_requirements": [
{
"comparator": "eq",
"version": "3.11.10"
}
]
}
]
}
},
"artifacts": {
"community_artifacts": {
"src": "$sources"
}
},
"runtime": {
"state_tool_artifacts": {
"src": "$artifacts",
"build_flags": []
}
},
"in": "$runtime"
}
}
3 changes: 2 additions & 1 deletion pkg/buildscript/unmarshal_buildexpression.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import (
"strings"
"time"

"github.com/go-openapi/strfmt"
"golang.org/x/text/cases"
"golang.org/x/text/language"

"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/go-openapi/strfmt"
)

// At this time, there is no way to ask the Platform for an empty build expression.
Expand Down Expand Up @@ -76,6 +76,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))
} else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" {
atTimeNode.Ident = ptr.To("TIME")
Expand Down
2 changes: 1 addition & 1 deletion pkg/platform/model/buildplanner/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (b *BuildPlanner) FetchCommit(commitID strfmt.UUID, owner, project string,
if err := script.UnmarshalBuildExpression(commit.Expression); err != nil {
return nil, errs.Wrap(err, "failed to parse build expression")
}
script.SetAtTime(time.Time(commit.AtTime))
script.SetAtTime(time.Time(commit.AtTime), false)

return &Commit{commit, bp, script}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/platform/model/buildplanner/buildscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript
}

script := buildscript.New()
script.SetAtTime(time.Time(resp.Commit.AtTime))
script.SetAtTime(time.Time(resp.Commit.AtTime), false)
if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil {
return nil, errs.Wrap(err, "failed to parse build expression")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/platform/model/buildplanner/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (b *BuildPlanner) StageCommit(params StageCommitParams) (*Commit, error) {
}

stagedScript := buildscript.New()
stagedScript.SetAtTime(time.Time(resp.Commit.AtTime))
stagedScript.SetAtTime(time.Time(resp.Commit.AtTime), false)
if err := stagedScript.UnmarshalBuildExpression(resp.Commit.Expression); err != nil {
return nil, errs.Wrap(err, "failed to parse build expression")
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/to-buildscript/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func main() {
bs := buildscript.New()
bs.SetProject(project)
if atTime != nil {
bs.SetAtTime(*atTime)
bs.SetAtTime(*atTime, true)
}
err := bs.UnmarshalBuildExpression([]byte(input))
if err != nil {
Expand Down
Loading