Skip to content

Commit

Permalink
gitlab: Create status with pr name
Browse files Browse the repository at this point in the history
In gitlab the check names where showing up as a single one for every
pipelines.

This would cause issue when there is multiple pipelinerun running and
one of them fails. If other success it would override it and would show
as success in the Pipeline.

We now create each status like in github with the applicationName
/ OriginalPipelineName

Signed-off-by: Chmouel Boudjnah <[email protected]>
  • Loading branch information
chmouel committed Dec 6, 2024
1 parent 425d4f2 commit 0315a46
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 76 deletions.
13 changes: 1 addition & 12 deletions pkg/provider/gitea/gitea.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts,
State: state,
TargetURL: status.DetailsURL,
Description: status.Title,
Context: getCheckName(status, pacopts),
Context: provider.GetCheckName(status, pacopts),
}
if _, _, err := v.Client.CreateStatus(event.Organization, event.Repository, event.SHA, gStatus); err != nil {
return err
Expand Down Expand Up @@ -197,17 +197,6 @@ func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts,
return nil
}

// TODO: move to common since used in github and here.
func getCheckName(status provider.StatusOpts, pacopts *info.PacOpts) string {
if pacopts.ApplicationName != "" {
if status.OriginalPipelineRunName == "" {
return pacopts.ApplicationName
}
return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName)
}
return status.OriginalPipelineRunName
}

func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, provenance string) (string, error) {
// default set provenance from the SHA
revision := event.SHA
Expand Down
16 changes: 3 additions & 13 deletions pkg/provider/github/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,6 @@ const taskStatusTemplate = `
{{- end }}
</table>`

func getCheckName(status provider.StatusOpts, pacopts *info.PacOpts) string {
if pacopts.ApplicationName != "" {
if status.OriginalPipelineRunName == "" {
return pacopts.ApplicationName
}
return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName)
}
return status.OriginalPipelineRunName
}

func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) {
opt := github.ListOptions{PerPage: v.PaginedNumber}
for {
Expand Down Expand Up @@ -114,7 +104,7 @@ func (v *Provider) canIUseCheckrunID(checkrunid *int64) bool {
func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) {
now := github.Timestamp{Time: time.Now()}
checkrunoption := github.CreateCheckRunOptions{
Name: getCheckName(status, v.pacInfo),
Name: provider.GetCheckName(status, v.pacInfo),
HeadSHA: runevent.SHA,
Status: github.String("in_progress"),
DetailsURL: github.String(status.DetailsURL),
Expand Down Expand Up @@ -254,7 +244,7 @@ func (v *Provider) getOrUpdateCheckRunStatus(ctx context.Context, runevent *info
checkRunOutput.Text = github.String(text)

opts := github.UpdateCheckRunOptions{
Name: getCheckName(statusOpts, pacopts),
Name: provider.GetCheckName(statusOpts, pacopts),
Status: github.String(statusOpts.Status),
Output: checkRunOutput,
}
Expand Down Expand Up @@ -323,7 +313,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, runevent *info.Event,
State: github.String(status.Conclusion),
TargetURL: github.String(status.DetailsURL),
Description: github.String(status.Title),
Context: github.String(getCheckName(status, v.pacInfo)),
Context: github.String(provider.GetCheckName(status, v.pacInfo)),
CreatedAt: &github.Timestamp{Time: now},
}

Expand Down
50 changes: 0 additions & 50 deletions pkg/provider/github/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,56 +561,6 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) {
}
}

func TestGetCheckName(t *testing.T) {
type args struct {
status provider.StatusOpts
pacopts *info.PacOpts
}
tests := []struct {
name string
args args
want string
}{
{
name: "no application name",
args: args{
status: provider.StatusOpts{
OriginalPipelineRunName: "HELLO",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: ""}},
},
want: "HELLO",
},
{
name: "application and pipelinerun name",
args: args{
status: provider.StatusOpts{
OriginalPipelineRunName: "MOTO",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "HELLO"}},
},
want: "HELLO / MOTO",
},
{
name: "application no pipelinerun name",
args: args{
status: provider.StatusOpts{
OriginalPipelineRunName: "",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "PAC"}},
},
want: "PAC",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getCheckName(tt.args.status, tt.args.pacopts); got != tt.want {
t.Errorf("getCheckName() = %v, want %v", got, tt.want)
}
})
}
}

func TestProviderGetExistingCheckRunID(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 3 additions & 1 deletion pkg/provider/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,13 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
body := fmt.Sprintf("**%s%s** has %s\n\n%s\n\n<small>Full log available [here](%s)</small>",
v.pacInfo.ApplicationName, onPr, statusOpts.Title, statusOpts.Text, detailsURL)

contextName := provider.GetCheckName(statusOpts, v.pacInfo)
opt := &gitlab.SetCommitStatusOptions{
State: gitlab.BuildStateValue(statusOpts.Conclusion),
Name: gitlab.Ptr(v.pacInfo.ApplicationName),
Name: gitlab.Ptr(contextName),
TargetURL: gitlab.Ptr(detailsURL),
Description: gitlab.Ptr(statusOpts.Title),
Context: gitlab.Ptr(contextName),
}

// In case we have access, set the status. Typically, on a Merge Request (MR)
Expand Down
17 changes: 17 additions & 0 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"regexp"
"strings"

"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
"gopkg.in/yaml.v2"
)

Expand Down Expand Up @@ -133,3 +134,19 @@ func ValidateYaml(content []byte, filename string) error {
}
return nil
}

// GetCheckName returns the name of the check to be created based on the status
// and the pacopts.
// If the pacopts.ApplicationName is set, it will be used as the check name.
// Otherwise, the OriginalPipelineRunName will be used.
// If the OriginalPipelineRunName is not set, an empty string will be returned.
// The check name will be in the format "ApplicationName / OriginalPipelineRunName".
func GetCheckName(status StatusOpts, pacopts *info.PacOpts) string {
if pacopts.ApplicationName != "" {
if status.OriginalPipelineRunName == "" {
return pacopts.ApplicationName
}
return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName)
}
return status.OriginalPipelineRunName
}
52 changes: 52 additions & 0 deletions pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package provider
import (
"testing"

"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
"gotest.tools/v3/assert"
)

Expand Down Expand Up @@ -494,3 +496,53 @@ func TestCompareHostOfURLS(t *testing.T) {
})
}
}

func TestGetCheckName(t *testing.T) {
type args struct {
status StatusOpts
pacopts *info.PacOpts
}
tests := []struct {
name string
args args
want string
}{
{
name: "no application name",
args: args{
status: StatusOpts{
OriginalPipelineRunName: "HELLO",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: ""}},
},
want: "HELLO",
},
{
name: "application and pipelinerun name",
args: args{
status: StatusOpts{
OriginalPipelineRunName: "MOTO",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "HELLO"}},
},
want: "HELLO / MOTO",
},
{
name: "application no pipelinerun name",
args: args{
status: StatusOpts{
OriginalPipelineRunName: "",
},
pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "PAC"}},
},
want: "PAC",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := GetCheckName(tt.args.status, tt.args.pacopts); got != tt.want {
t.Errorf("GetCheckName() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 0315a46

Please sign in to comment.