-
Notifications
You must be signed in to change notification settings - Fork 13
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
State revert uses the build planner #2858
Changes from 6 commits
c0ad8fa
e7b1401
69aa7d1
824ecad
b251994
f6fb72d
1611afe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
package request | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, I understand this now! |
||
|
||
import "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" | ||
|
||
func RevertCommit(organization, project, targetVcsRef, commitID string) *revertCommit { | ||
return &revertCommit{map[string]interface{}{ | ||
"organization": organization, | ||
"project": project, | ||
"targetVcsRef": targetVcsRef, | ||
"commitId": commitID, | ||
// Currently, we use the force strategy for all revert commits. | ||
// This is because we don't have a way to show the user the conflicts | ||
// and let them resolve them yet. | ||
// https://activestatef.atlassian.net/browse/AR-80?focusedCommentId=46998 | ||
"strategy": model.RevertCommitStrategyForce, | ||
}} | ||
} | ||
|
||
type revertCommit struct { | ||
vars map[string]interface{} | ||
} | ||
|
||
func (r *revertCommit) Query() string { | ||
return ` | ||
mutation ($organization: String!, $project: String!, $commitId: String!, $targetVcsRef: String!, $strategy: RevertStrategy) { | ||
revertCommit( | ||
input: {organization: $organization, project: $project, commitId: $commitId, targetVcsRef: $targetVcsRef, strategy: $strategy} | ||
) { | ||
... on RevertedCommit { | ||
__typename | ||
commit { | ||
__typename | ||
commitId | ||
} | ||
} | ||
... on RevertConflict { | ||
__typename | ||
message | ||
commitId | ||
targetCommitId | ||
conflictPaths | ||
} | ||
... on CommitHasNoParent { | ||
__typename | ||
message | ||
} | ||
... on NotFound { | ||
__typename | ||
message | ||
mayNeedAuthentication | ||
} | ||
... on ParseError { | ||
__typename | ||
message | ||
path | ||
} | ||
... on ValidationError { | ||
__typename | ||
message | ||
} | ||
... on Forbidden { | ||
__typename | ||
message | ||
} | ||
... on HeadOnBranchMoved { | ||
__typename | ||
message | ||
} | ||
... on NoChangeSinceLastCommit { | ||
message | ||
commitId | ||
} | ||
} | ||
}` | ||
} | ||
|
||
func (r *revertCommit) Vars() map[string]interface{} { | ||
return r.vars | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,7 @@ func (suite *RevertIntegrationTestSuite) TestRevert() { | |
e2e.OptArgs("history"), | ||
e2e.OptWD(wd), | ||
) | ||
cp.Expect("Revert commit " + commitID) | ||
cp.Expect("Reverted commit for commit " + commitID) | ||
cp.Expect("- urllib3") | ||
cp.Expect("+ argparse") // parent commit | ||
cp.Expect("+ urllib3") // commit whose changes were just reverted | ||
|
@@ -83,7 +83,7 @@ func (suite *RevertIntegrationTestSuite) TestRevert_failsOnCommitNotInHistory() | |
cp.Expect(fmt.Sprintf("Operating on project %s", namespace)) | ||
cp.SendLine("Y") | ||
cp.Expect(commitID) | ||
cp.Expect("The commit being reverted is not within the current commit's history") | ||
cp.Expect("The target commit is not within the current commit's history") | ||
cp.ExpectNotExitCode(0) | ||
} | ||
|
||
|
@@ -115,11 +115,12 @@ func (suite *RevertIntegrationTestSuite) TestRevertTo() { | |
e2e.OptArgs("history"), | ||
e2e.OptWD(wd), | ||
) | ||
cp.Expect("Reverting to commit " + commitID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is printed to the user in this case? Should we be asserting it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, this is actually the commit message that is now different with the buildplanner |
||
cp.Expect("Revert to commit " + commitID) | ||
cp.Expect("- argparse") // effectively reverting previous commit | ||
cp.Expect("+ argparse") // commit being effectively reverted | ||
cp.Expect("+ urllib3") // commit reverted to | ||
cp.Expect("+ python") // initial commit | ||
fmt.Println("Output: ", cp.Snapshot()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was accidentally left here instead of removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, thanks for catching that! |
||
} | ||
|
||
func (suite *RevertIntegrationTestSuite) TestRevertTo_failsOnCommitNotInHistory() { | ||
|
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.
Good call to swap these two.