Skip to content

Commit f5f8567

Browse files
authored
chore: Introduce unified experimental handling (#386)
1 parent da9c13f commit f5f8567

File tree

5 files changed

+167
-11
lines changed

5 files changed

+167
-11
lines changed

pkg/local_workflows/code_workflow.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ func GetCodeFlagSet() *pflag.FlagSet {
3737
flagSet.String(code_workflow.ConfigurationTargetName, "", "The name of the target to test.")
3838
flagSet.String(code_workflow.ConfigurationTargetReference, "", "The reference that differentiates this project, e.g. a branch name or version.")
3939
flagSet.String("target-file", "", "The path to the target file to test.")
40-
flagSet.Bool(configuration.FLAG_EXPERIMENTAL, false, "Enable experimental code test command")
4140

4241
return flagSet
4342
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package config_utils
2+
3+
import (
4+
"strings"
5+
6+
"github.com/spf13/pflag"
7+
8+
"github.com/snyk/go-application-framework/pkg/configuration"
9+
)
10+
11+
func IsExperimental(flags *pflag.FlagSet) bool {
12+
result := false
13+
14+
// if the experimental flag exists, it's an experimental command
15+
if tmp := flags.Lookup(configuration.FLAG_EXPERIMENTAL); tmp != nil {
16+
result = true
17+
18+
// if the experimental flag is deprecated, it'll be ignored
19+
if strings.Contains(strings.ToLower(tmp.Usage), "deprecated") ||
20+
len(tmp.Deprecated) > 0 {
21+
result = false
22+
}
23+
}
24+
25+
return result
26+
}
27+
28+
func MarkAsExperimental(flags *pflag.FlagSet) *pflag.FlagSet {
29+
if flags == nil {
30+
return nil
31+
}
32+
33+
result := *flags
34+
if result.Lookup(configuration.FLAG_EXPERIMENTAL) == nil {
35+
result.Bool(configuration.FLAG_EXPERIMENTAL, false, "enable experimental command")
36+
}
37+
return &result
38+
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package config_utils
2+
3+
import (
4+
"testing"
5+
6+
"github.com/spf13/pflag"
7+
"github.com/stretchr/testify/assert"
8+
9+
"github.com/snyk/go-application-framework/pkg/configuration"
10+
)
11+
12+
func TestIsExperimental(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
setupFlags func(t *testing.T) *pflag.FlagSet
16+
expectedResult bool
17+
}{
18+
{
19+
name: "returns false when experimental flag does not exist",
20+
setupFlags: func(_ *testing.T) *pflag.FlagSet {
21+
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
22+
flags.String("other-flag", "", "some other flag")
23+
return flags
24+
},
25+
expectedResult: false,
26+
},
27+
{
28+
name: "returns true when experimental flag exists and is not deprecated",
29+
setupFlags: func(_ *testing.T) *pflag.FlagSet {
30+
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
31+
return MarkAsExperimental(flags)
32+
},
33+
expectedResult: true,
34+
},
35+
{
36+
name: "returns false when experimental flag usage contains 'deprecated' (case insensitive)",
37+
setupFlags: func(_ *testing.T) *pflag.FlagSet {
38+
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
39+
flags.Bool(configuration.FLAG_EXPERIMENTAL, false, "This is DEPRECATED and should not be used")
40+
return flags
41+
},
42+
expectedResult: false,
43+
},
44+
{
45+
name: "returns false when experimental flag deprecated field is set",
46+
setupFlags: func(t *testing.T) *pflag.FlagSet {
47+
t.Helper()
48+
flags := MarkAsExperimental(pflag.NewFlagSet("test", pflag.ContinueOnError))
49+
err := flags.MarkDeprecated(configuration.FLAG_EXPERIMENTAL, "use new-flag instead")
50+
assert.NoError(t, err)
51+
return flags
52+
},
53+
expectedResult: false,
54+
},
55+
}
56+
57+
for _, tt := range tests {
58+
t.Run(tt.name, func(t *testing.T) {
59+
flags := tt.setupFlags(t)
60+
result := IsExperimental(flags)
61+
assert.Equal(t, tt.expectedResult, result)
62+
})
63+
}
64+
}
65+
66+
func TestMarkAsExperimental(t *testing.T) {
67+
t.Run("makes non-experimental flagset experimental", func(t *testing.T) {
68+
original := pflag.NewFlagSet("test", pflag.ContinueOnError)
69+
original.String("existing-flag", "", "existing flag")
70+
71+
result := MarkAsExperimental(original)
72+
73+
// Should make the flagset experimental
74+
assert.True(t, IsExperimental(result), "flagset should be experimental after marking")
75+
})
76+
77+
t.Run("returns different flagset instance", func(t *testing.T) {
78+
original := pflag.NewFlagSet("test", pflag.ContinueOnError)
79+
80+
result := MarkAsExperimental(original)
81+
82+
// Should return a different instance
83+
assert.NotSame(t, original, result, "should return a new flagset instance")
84+
85+
// Original should not be experimental, result should be
86+
assert.False(t, IsExperimental(original), "original flagset should not be experimental")
87+
assert.True(t, IsExperimental(result), "result flagset should be experimental")
88+
})
89+
90+
t.Run("makes empty flagset experimental", func(t *testing.T) {
91+
original := pflag.NewFlagSet("empty", pflag.ContinueOnError)
92+
93+
result := MarkAsExperimental(original)
94+
95+
// Should make empty flagset experimental
96+
assert.True(t, IsExperimental(result), "empty flagset should be experimental after marking")
97+
})
98+
99+
t.Run("preserves existing experimental flag", func(t *testing.T) {
100+
original := pflag.NewFlagSet("test", pflag.ContinueOnError)
101+
original.Bool(configuration.FLAG_EXPERIMENTAL, true, "old usage")
102+
103+
result := MarkAsExperimental(original)
104+
105+
// Should remain experimental
106+
assert.True(t, IsExperimental(result), "should remain experimental")
107+
})
108+
109+
t.Run("works with both new and existing experimental flags", func(t *testing.T) {
110+
// Test with flagset that doesn't have experimental flag
111+
original1 := pflag.NewFlagSet("test1", pflag.ContinueOnError)
112+
original1.String("other-flag", "", "some other flag")
113+
114+
result1 := MarkAsExperimental(original1)
115+
116+
// Should be experimental after adding flag
117+
assert.True(t, IsExperimental(result1), "should be experimental after adding flag")
118+
119+
// Test with flagset that already has experimental flag (reuse result1)
120+
result2 := MarkAsExperimental(result1)
121+
122+
// Should remain experimental
123+
assert.True(t, IsExperimental(result2), "should remain experimental")
124+
})
125+
}

pkg/local_workflows/connectivity_check_extension/connectivity_check_workflow.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import (
77
"os"
88

99
"github.com/rs/zerolog"
10-
"github.com/snyk/error-catalog-golang-public/cli"
1110
"github.com/spf13/pflag"
1211
"golang.org/x/term"
1312

1413
"github.com/snyk/go-application-framework/pkg/configuration"
14+
"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
1515
"github.com/snyk/go-application-framework/pkg/local_workflows/connectivity_check_extension/connectivity"
1616
"github.com/snyk/go-application-framework/pkg/workflow"
1717
)
@@ -35,9 +35,8 @@ func InitConnectivityCheckWorkflow(engine workflow.Engine) error {
3535
config.Bool(noColorFlag, false, "Disable colored output")
3636
config.Int(timeoutFlag, 10, "Timeout in seconds for each connection test")
3737
config.Int(maxOrgCountFlag, 100, "Maximum number of organizations to retrieve")
38-
config.Bool(configuration.FLAG_EXPERIMENTAL, false, "This feature is experimental")
3938

40-
_, err := engine.Register(WORKFLOWID_CONNECTIVITY_CHECK, workflow.ConfigurationOptionsFromFlagset(config), connectivityCheckEntryPoint)
39+
_, err := engine.Register(WORKFLOWID_CONNECTIVITY_CHECK, workflow.ConfigurationOptionsFromFlagset(config_utils.MarkAsExperimental(config)), connectivityCheckEntryPoint)
4140
return err
4241
}
4342

@@ -48,10 +47,6 @@ func connectivityCheckEntryPoint(invocationCtx workflow.InvocationContext, input
4847
networkAccess := invocationCtx.GetNetworkAccess()
4948
ui := invocationCtx.GetUserInterface()
5049

51-
if !config.GetBool(configuration.FLAG_EXPERIMENTAL) {
52-
return nil, cli.NewCommandIsExperimentalError("")
53-
}
54-
5550
checker := connectivity.NewChecker(networkAccess, logger, config, ui)
5651

5752
logger.Info().Msg("Starting Snyk connectivity check")

pkg/local_workflows/whoami_workflow.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/snyk/go-application-framework/internal/api"
1111
"github.com/snyk/go-application-framework/internal/api/contract"
1212
"github.com/snyk/go-application-framework/pkg/configuration"
13+
"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
1314
"github.com/snyk/go-application-framework/pkg/workflow"
1415
)
1516

@@ -26,13 +27,11 @@ var WORKFLOWID_WHOAMI workflow.Identifier = workflow.NewWorkflowIdentifier(whoAm
2627
func InitWhoAmIWorkflow(engine workflow.Engine) error {
2728
// initialize workflow configuration
2829
whoAmIConfig := pflag.NewFlagSet(whoAmIworkflowName, pflag.ExitOnError)
29-
// add experimental flag to configuration
30-
whoAmIConfig.Bool(experimentalFlag, false, "enable experimental whoAmI command")
3130
// add json flag to configuration
3231
whoAmIConfig.Bool(jsonFlag, false, "output in json format")
3332

3433
// register workflow with engine
35-
_, err := engine.Register(WORKFLOWID_WHOAMI, workflow.ConfigurationOptionsFromFlagset(whoAmIConfig), whoAmIWorkflowEntryPoint)
34+
_, err := engine.Register(WORKFLOWID_WHOAMI, workflow.ConfigurationOptionsFromFlagset(config_utils.MarkAsExperimental(whoAmIConfig)), whoAmIWorkflowEntryPoint)
3635
return err
3736
}
3837

0 commit comments

Comments
 (0)