Advanced trigger map (#932)

* Introduce new trigger item fields and update validation * Note some todos * Start introducing new trigger condition type * Validate trigger item types * Code cleanup * Update trigger map existing tests * Skip some lint checks * Update trigger item type names * Fix regex condition value type cast * Fix regex condition value handling * Fix regex value * Fix new fields' missing omitempty * Trigger item type is not necessarily required * Enabled is bool pointer * Trigger map validation integration tests * Unit test trigger items' validate func * Normalize trigger map * Update mixed type error message and add tests * Update trigger map normalize error message * Bump bitrise.yml format version * Code cleanup * Validate type field * Fix version test * Update trigger map validation error messages * Typo fix (Normalise -> Normalize) --------- Co-authored-by: Krisztián Gödrei <[email protected]>
bitrise-io · Mar 26, 2024 · 6bcf325 · 6bcf325
1 parent e8a28a7
commit 6bcf325
Show file tree

Hide file tree

Showing 8 changed files with 1,027 additions and 175 deletions.
diff --git a/_tests/integration/version_test.go b/_tests/integration/version_test.go
@@ -8,6 +8,8 @@ import (
  "runtime"
  "testing"
 
+ "github.com/bitrise-io/bitrise/models"
+
  "github.com/bitrise-io/bitrise/version"
  "github.com/bitrise-io/go-utils/command"
  "github.com/stretchr/testify/require"
@@ -21,11 +23,11 @@ func Test_VersionOutput(t *testing.T) {
 
  expectedOSVersion := fmt.Sprintf("%s (%s)", runtime.GOOS, runtime.GOARCH)
  expectedVersionOut := fmt.Sprintf(`version: %s
-format version: 13
+format version: %s
 os: %s
 go: %s
 build number: 
-commit: (devel)`, version.VERSION, expectedOSVersion, runtime.Version())
+commit: (devel)`, version.VERSION, models.FormatVersion, expectedOSVersion, runtime.Version())
 
  require.Equal(t, expectedVersionOut, out)
  }

diff --git a/models/models.go b/models/models.go
@@ -11,7 +11,7 @@ import (
 
 const (
  // FormatVersion ...
- FormatVersion = "13"
+ FormatVersion = "14"
 )
 
 // StepListItemModel ...

diff --git a/models/models_methods.go b/models/models_methods.go
@@ -142,6 +142,12 @@ func (config *BitriseDataModel) Normalize() error {
  return err
  }
 
+ normalizedTriggerMap, err := config.TriggerMap.Normalized()
+ if err != nil {
+ return err
+ }
+ config.TriggerMap = normalizedTriggerMap
+
  for _, workflow := range config.Workflows {
  if err := workflow.Normalize(); err != nil {
  return err

diff --git a/models/models_methods_test.go b/models/models_methods_test.go
@@ -1,6 +1,7 @@
 package models
 
 import (
+ "encoding/json"
  "os"
  "strings"
  "testing"
@@ -16,6 +17,78 @@ import (
 // ----------------------------
 // --- Validate
 
+func TestJSONMarshal(t *testing.T) {
+ tests := []struct {
+ name string
+ config string
+ wantWarns []string
+ wantErr string
+ }{
+ {
+ name: "Step inputs are normalized",
+ config: `format_version: "13"
+default_step_lib_source: "https:/bitrise-io/bitrise-steplib.git"
+
+workflows:
+ test:
+ steps:
+ - script:
+ inputs:
+ - content: 'echo "Hello $NAME!"'
+ opts:
+ is_expand: false`,
+ },
+ {
+ name: "Trigger map items are normalized",
+ config: `format_version: 13
+default_step_lib_source: "https:/bitrise-io/bitrise-steplib.git"
+
+trigger_map:
+- push_branch: main
+ enabled: false
+ workflow: deploy
+- push_branch: main
+ commit_message: "[workflow: test]"
+ workflow: test
+- push_branch: main
+ changed_files:
+ regex: '^ios\/.*'
+ workflow: ios-deploy
+- pull_request_target_branch: main
+ draft_pull_request_enabled: false
+ workflow: test
+- pull_request_target_branch: '*'
+ pull_request_label: "[workflow: check]"
+ workflow: check
+- tag: '*'
+ workflow: deploy
+
+workflows:
+ wip-deploy:
+ ios-deploy:
+ deploy:
+ test:
+ check:`,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ var config BitriseDataModel
+ require.NoError(t, yaml.Unmarshal([]byte(tt.config), &config))
+
+ warns, err := config.Validate()
+ require.Empty(t, warns)
+ require.NoError(t, err)
+
+ err = config.Normalize()
+ require.NoError(t, err)
+
+ _, err = json.Marshal(config)
+ require.NoError(t, err)
+ })
+ }
+}
+
 // Config
 func TestValidateConfig(t *testing.T) {
  t.Log("Valid bitriseData")
@@ -385,6 +458,109 @@ workflows:
 }
 
 // Trigger map
+func TestValidateTriggerMap(t *testing.T) {
+ tests := []struct {
+ name string
+ config string
+ wantWarns []string
+ wantErr string
+ }{
+ {
+ name: "valid legacy trigger map",
+ config: `
+format_version: 13
+default_step_lib_source: "https:/bitrise-io/bitrise-steplib.git"
+
+trigger_map:
+- pattern: main
+ workflow: deploy
+- pattern: "*"
+ is_pull_request_allowed: true
+ workflow: test
+
+workflows:
+ deploy:
+ test:
+`,
+ },
+ {
+ name: "valid trigger map",
+ config: `
+format_version: 13
+default_step_lib_source: "https:/bitrise-io/bitrise-steplib.git"
+
+trigger_map:
+- push_branch: main
+ workflow: deploy
+- pull_request_target_branch: main
+ workflow: test
+- pull_request_target_branch: '*'
+ workflow: check
+- tag: '*'
+ workflow: deploy
+
+workflows:
+ deploy:
+ test:
+ check:
+`,
+ },
+ {
+ name: "valid advanced trigger map",
+ config: `
+format_version: 13
+default_step_lib_source: "https:/bitrise-io/bitrise-steplib.git"
+
+trigger_map:
+- push_branch: main
+ enabled: false
+ workflow: deploy
+- push_branch: main
+ commit_message: "[workflow: test]"
+ workflow: test
+- push_branch: main
+ changed_files:
+ regex: '^ios\/.*'
+ workflow: ios-deploy
+- pull_request_target_branch: main
+ draft_pull_request_enabled: false
+ workflow: test
+- pull_request_target_branch: '*'
+ pull_request_label: "[workflow: check]"
+ workflow: check
+- tag: '*'
+ workflow: deploy
+
+workflows:
+ wip-deploy:
+ ios-deploy:
+ deploy:
+ test:
+ check:
+`,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ var config BitriseDataModel
+ require.NoError(t, yaml.Unmarshal([]byte(tt.config), &config))
+
+ warns, err := config.Validate()
+ if len(tt.wantWarns) > 0 {
+ require.Equal(t, tt.wantWarns, warns)
+ } else {
+ require.Empty(t, warns)
+ }
+
+ if tt.wantErr != "" {
+ require.EqualError(t, err, tt.wantErr)
+ } else {
+ require.NoError(t, err)
+ }
+ })
+ }
+}
+
 func TestTriggerMapItemValidate(t *testing.T) {
  t.Log("utility workflow triggered - Warning")
  {
@@ -405,7 +581,7 @@ workflows:
 
  warnings, err := config.Validate()
  require.NoError(t, err)
- require.Equal(t, []string{"workflow (_deps-update) defined in trigger item (push_branch: /release -> workflow: _deps-update), but utility workflows can't be triggered directly"}, warnings)
+ require.Equal(t, []string{"trigger item #1: utility workflow (_deps-update) defined as trigger target, but utility workflows can't be triggered directly"}, warnings)
  }
 
  t.Log("pipeline not exists")
@@ -436,7 +612,7 @@ workflows:
  require.NoError(t, err)
 
  _, err = config.Validate()
- require.EqualError(t, err, "pipeline (release) defined in trigger item (push_branch: /release -> pipeline: release), but does not exist")
+ require.EqualError(t, err, "trigger item #1: non-existent pipeline defined as trigger target: release")
  }
 
  t.Log("workflow not exists")
@@ -457,7 +633,7 @@ workflows:
  require.NoError(t, err)
 
  _, err = config.Validate()
- require.EqualError(t, err, "workflow (release) defined in trigger item (push_branch: /release -> workflow: release), but does not exist")
+ require.EqualError(t, err, "trigger item #1: non-existent workflow defined as trigger target: release")
  }
 }
 

diff --git a/models/trigger_map.go b/models/trigger_map.go
@@ -6,20 +6,33 @@ import (
 
 type TriggerMapModel []TriggerMapItemModel
 
-func (triggerMap TriggerMapModel) Validate(workflows, pipelines []string) ([]string, error) {
- var warnings []string
- for _, item := range triggerMap {
- warns, err := item.Validate(workflows, pipelines)
- warnings = append(warnings, warns...)
+func (triggerMap TriggerMapModel) Normalized() ([]TriggerMapItemModel, error) {
+ var items []TriggerMapItemModel
+ for idx, item := range triggerMap {
+ normalizedItem, err := item.Normalized(idx)
  if err != nil {
- return warnings, err
+ return nil, err
  }
+ items = append(items, normalizedItem)
  }
+ return items, nil
+}
+
+func (triggerMap TriggerMapModel) Validate(workflows, pipelines []string) ([]string, error) {
+ var warnings []string
 
  if err := triggerMap.checkDuplicatedTriggerMapItems(); err != nil {
  return warnings, err
  }
 
+ for idx, item := range triggerMap {
+ warns, err := item.Validate(idx, workflows, pipelines)
+ warnings = append(warnings, warns...)
+ if err != nil {
+ return warnings, err
+ }
+ }
+
  return warnings, nil
 }
 
@@ -38,17 +51,17 @@ func (triggerMap TriggerMapModel) FirstMatchingTarget(pushBranch, prSourceBranch
 }
 
 func (triggerMap TriggerMapModel) checkDuplicatedTriggerMapItems() error {
- items := make(map[string]struct{})
+ items := make(map[string]int)
 
- for _, triggerItem := range triggerMap {
- content := triggerItem.String(false)
+ for idx, triggerItem := range triggerMap {
+ itemStr := triggerItem.conditionsString()
 
- _, ok := items[content]
+ storedItemIdx, ok := items[itemStr]
  if ok {
- return fmt.Errorf("duplicated trigger item found (%s)", content)
+ return fmt.Errorf("the %d. trigger item duplicates the %d. trigger item", idx+1, storedItemIdx+1)
  }
 
- items[content] = struct{}{}
+ items[itemStr] = idx
  }
 
  return nil