Skip to content

Commit

Permalink
fix(provider): handle branch names with command keywords in pr comments
Browse files Browse the repository at this point in the history
This commit fixes a bug in parsing pipeline control comments (e.g., /test,
/retest, /cancel) when branch names contain command keywords like "/test".
Previously, the code would incorrectly split comments with such branch
names, causing failures.

Key improvements:
- Use `strings.SplitN` to split only on the first instance of the command
  keyword.
- Add support for comments with key=value pairs after the pipeline name.
- Include test cases for branch names containing "/test" and comments with
  `key=value` pairs.

Example:
Before: Branch "feature/test-new" would break parsing due to multiple splits
on "/test" keyword.
After: The code now correctly processes branch names with command keywords and
handles key=value pairs.

The parsing is now more robust for real-world scenarios.
  • Loading branch information
l-qing authored and chmouel committed Feb 4, 2025
1 parent af3a1bb commit 1bd4586
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
9 changes: 6 additions & 3 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,13 @@ func GetPipelineRunAndBranchNameFromCancelComment(comment string) (string, strin
// by /test, /retest or /cancel to return branch name and pipelinerun name.
func getPipelineRunAndBranchNameFromComment(typeOfComment, comment string) (string, string, error) {
var prName, branchName string
splitTest := strings.Split(comment, typeOfComment)
// avoid parsing error due to branch name contains /test, /retest or /cancel,
// here only split the first keyword and not split the later keywords.
splitTest := strings.SplitN(comment, typeOfComment, 2)

// after the split get the second part of the typeOfComment (/test, /retest or /cancel)
// as second part can be branch name or pipelinerun name and branch name
// ex: /test branch:nightly, /test prname branch:nightly
// ex: /test branch:nightly, /test prname branch:nightly, /test prname branch:nightly key=value
if splitTest[1] != "" && strings.Contains(splitTest[1], ":") {
branchData := strings.Split(splitTest[1], ":")

Expand All @@ -108,7 +110,8 @@ func getPipelineRunAndBranchNameFromComment(typeOfComment, comment string) (stri
// ex: /test prname
getFirstLine := strings.Split(splitTest[1], "\n")
// trim spaces
prName = strings.TrimSpace(getFirstLine[0])
// adapt for the comment contains the key=value pair
prName = strings.Split(strings.TrimSpace(getFirstLine[0]), " ")[0]
}
return prName, branchName, nil
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,13 @@ func TestGetPipelineRunAndBranchNameFromTestComment(t *testing.T) {
branchName: "test",
wantError: false,
},
{
name: "branch name contains /test",
comment: "/test abc-01-pr abc \n branch:chore/test",
prName: "abc-01-pr",
branchName: "chore/test",
wantError: false,
},
{
name: "different word other than branch for retest command",
comment: "/retest invalidname:nightly",
Expand All @@ -341,6 +348,13 @@ func TestGetPipelineRunAndBranchNameFromTestComment(t *testing.T) {
prName: "abc-01-pr",
wantError: false,
},
{
name: "test a pipeline with key value",
comment: "/test abc-01-pr key=value",
prName: "abc-01-pr",
branchName: "",
wantError: false,
},
{
name: "string before retest command",
comment: "abc \n /retest abc-01-pr",
Expand Down

0 comments on commit 1bd4586

Please sign in to comment.