-
Notifications
You must be signed in to change notification settings - Fork 30
feat: migrate experiments config from array to bools #398
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
base: main
Are you sure you want to change the base?
Changes from all commits
4a0fe07
95d3cd1
f01e37f
b288fb4
47f6207
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 |
|---|---|---|
|
|
@@ -17,7 +17,9 @@ package config | |
| import ( | ||
| "context" | ||
| "fmt" | ||
| "slices" | ||
| "maps" | ||
| "sort" | ||
| "strings" | ||
|
|
||
| "github.com/slackapi/slack-cli/internal/experiment" | ||
| "github.com/slackapi/slack-cli/internal/slackerror" | ||
|
|
@@ -29,47 +31,73 @@ func (c *Config) LoadExperiments( | |
| ctx context.Context, | ||
| printDebug func(ctx context.Context, format string, a ...interface{}), | ||
| ) { | ||
| experiments := []experiment.Experiment{} | ||
| experiments := map[string]bool{} | ||
| // Load from flags | ||
| for _, flagValue := range c.ExperimentsFlag { | ||
| experiments = append(experiments, experiment.Experiment(flagValue)) | ||
| experiments[flagValue] = true | ||
| } | ||
|
Comment on lines
35
to
38
Member
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. 👁️🗨️ thought: The order of experiment parsing might need to be reversed so that project and user configurations can be overridden with a flag if the lower settings use a "false" value? |
||
| printDebug(ctx, fmt.Sprintf("active flag experiments: %s", experiments)) | ||
| printDebug(ctx, fmt.Sprintf("active flag experiments: %s", formatExperimentMap(experiments))) | ||
| // Load from project config file | ||
| projectConfig, err := ReadProjectConfigFile(ctx, c.fs, c.os) | ||
| if err != nil && slackerror.ToSlackError(err).Code != slackerror.ErrInvalidAppDirectory { | ||
| printDebug(ctx, fmt.Sprintf("failed to parse project-level config file: %s", err)) | ||
| } else if err == nil { | ||
| printDebug(ctx, fmt.Sprintf("active project experiments: %s", projectConfig.Experiments)) | ||
| experiments = append(experiments, projectConfig.Experiments...) | ||
| printDebug(ctx, fmt.Sprintf("active project experiments: %s", formatExperimentMap(projectConfig.Experiments))) | ||
| maps.Copy(experiments, projectConfig.Experiments) | ||
| } | ||
| // Load from system config file | ||
| userConfig, err := c.SystemConfig.UserConfig(ctx) | ||
| if err != nil { | ||
| printDebug(ctx, fmt.Sprintf("failed to parse system-level config file: %s", err)) | ||
| } else { | ||
| printDebug(ctx, fmt.Sprintf("active system experiments: %s", userConfig.Experiments)) | ||
| experiments = append(experiments, userConfig.Experiments...) | ||
| printDebug(ctx, fmt.Sprintf("active system experiments: %s", formatExperimentMap(userConfig.Experiments))) | ||
| maps.Copy(experiments, userConfig.Experiments) | ||
| } | ||
| // Load from permanently enabled list | ||
| experiments = append(experiments, experiment.EnabledExperiments...) | ||
| for _, exp := range experiment.EnabledExperiments { | ||
| experiments[string(exp)] = true | ||
| } | ||
| printDebug(ctx, fmt.Sprintf("active permanently enabled experiments: %s", experiment.EnabledExperiments)) | ||
| // Sort, trim, and audit the experiments list | ||
| slices.Sort(experiments) | ||
| c.experiments = slices.Compact(experiments) | ||
| for _, exp := range c.experiments { | ||
| if !experiment.Includes(exp) { | ||
| printDebug(ctx, fmt.Sprintf("invalid experiment found: %s", exp)) | ||
| // Audit the experiments | ||
| c.experiments = experiments | ||
| for name := range c.experiments { | ||
| if !experiment.Includes(experiment.Experiment(name)) { | ||
| printDebug(ctx, fmt.Sprintf("invalid experiment found: %s", name)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // GetExperiments returns the set of active experiments | ||
| func (c *Config) GetExperiments() []experiment.Experiment { | ||
| return c.experiments | ||
| result := make([]experiment.Experiment, 0, len(c.experiments)) | ||
| for name, enabled := range c.experiments { | ||
| if enabled { | ||
| result = append(result, experiment.Experiment(name)) | ||
| } | ||
| } | ||
| sort.Slice(result, func(i, j int) bool { | ||
| return result[i] < result[j] | ||
| }) | ||
| return result | ||
| } | ||
|
|
||
| // WithExperimentOn checks whether an experiment is currently toggled on | ||
| func (c *Config) WithExperimentOn(experimentToCheck experiment.Experiment) bool { | ||
| return slices.Contains(c.experiments, experimentToCheck) | ||
| return c.experiments[string(experimentToCheck)] | ||
| } | ||
|
|
||
| // formatExperimentMap returns a string representation of the experiments map | ||
| // for debug logging, formatted similar to the old slice format. | ||
| func formatExperimentMap(m map[string]bool) string { | ||
| if len(m) == 0 { | ||
| return "[]" | ||
| } | ||
| names := make([]string, 0, len(m)) | ||
| for name, enabled := range m { | ||
| if enabled { | ||
| names = append(names, name) | ||
| } | ||
| } | ||
| sort.Strings(names) | ||
| return "[" + strings.Join(names, " ") + "]" | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,6 @@ import ( | |||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||
| "path/filepath" | ||||||||||||||||||||||||||||||||||||||||
| "slices" | ||||||||||||||||||||||||||||||||||||||||
| "testing" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| "github.com/slackapi/slack-cli/internal/experiment" | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -40,7 +39,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Write our test script to the memory file system used by the mock | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", validExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, validExperiment)) | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
-43
to
+42
Member
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. ⭐ praise: Thanks for improving both the string formatting and |
||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,13 +49,33 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf("active system experiments: [%s]", validExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| t.Run("Correctly finds experiments from old array format in config.json", func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||
| // Setup | ||||||||||||||||||||||||||||||||||||||||
| ctx, fs, _, config, pathToConfigJSON, teardown := setup(t) | ||||||||||||||||||||||||||||||||||||||||
| defer teardown(t) | ||||||||||||||||||||||||||||||||||||||||
| _, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Write old array format | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte( | ||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf( | ||||||||||||||||||||||||||||||||||||||||
| `{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":["%s"]}`, | ||||||||||||||||||||||||||||||||||||||||
| validExperiment, | ||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
| experimentOn := config.WithExperimentOn(validExperiment) | ||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, true, experimentOn) | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+52
to
+71
Member
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.
Suggested change
🪓 quibble(non-blocking): We might not fear breaking experimental configuration setups since this can become burdensome to keep in mind ongoing? |
||||||||||||||||||||||||||||||||||||||||
| t.Run("Correctly returns false when experiments are not in config.json", func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||
| // Setup | ||||||||||||||||||||||||||||||||||||||||
| ctx, fs, _, config, pathToConfigJSON, teardown := setup(t) | ||||||||||||||||||||||||||||||||||||||||
| defer teardown(t) | ||||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[]}") | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{}}`) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -72,7 +91,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Write no contents via config.json | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[]}") | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{}}`) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
| config.ExperimentsFlag = []string{string(validExperiment)} | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -89,7 +108,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| defer teardown(t) | ||||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", invalidExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, invalidExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, mockOutput.String(), | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -120,7 +139,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Write contents via config.json | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", tc.experiment)) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, tc.experiment)) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -172,7 +191,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf(`{"experiments":["%s"]}`, experiment.Placeholder)) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf(`{"experiments":{"%s":true}}`, experiment.Placeholder)) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -185,36 +204,37 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| t.Run("Loads valid experiments from project configs and removes duplicates", func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||
| ctx, fs, _, config, _, teardown := setup(t) | ||||||||||||||||||||||||||||||||||||||||
| defer teardown(t) | ||||||||||||||||||||||||||||||||||||||||
| mockOutput, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
| _, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
| err := fs.Mkdir(slackdeps.MockWorkingDirectory, 0755) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| err = fs.Mkdir(filepath.Join(slackdeps.MockWorkingDirectory, ProjectConfigDirName), 0755) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf( | ||||||||||||||||||||||||||||||||||||||||
| `{"experiments":["%s", "%s", "%s"]}`, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte( | ||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf(`{"experiments":{"%s":true, "%s":true, "%s":true}}`, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Also set via flag to test dedup | ||||||||||||||||||||||||||||||||||||||||
| config.ExperimentsFlag = []string{string(experiment.Placeholder)} | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| config.LoadExperiments(ctx, mockPrintDebug) | ||||||||||||||||||||||||||||||||||||||||
| assert.True(t, config.WithExperimentOn(experiment.Placeholder)) | ||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, mockOutput.String(), fmt.Sprintf( | ||||||||||||||||||||||||||||||||||||||||
| "active project experiments: [%s %s %s]", | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| experiment.Placeholder, | ||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||
| // Assert that duplicates are removed and slice length is reduced | ||||||||||||||||||||||||||||||||||||||||
| // Add in the permanently enabled experiments before comparing | ||||||||||||||||||||||||||||||||||||||||
| activeExperiments := append(experiment.EnabledExperiments, experiment.Placeholder) | ||||||||||||||||||||||||||||||||||||||||
| slices.Sort(activeExperiments) | ||||||||||||||||||||||||||||||||||||||||
| activeExperiments = slices.Compact(activeExperiments) | ||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, activeExperiments, config.experiments) | ||||||||||||||||||||||||||||||||||||||||
| // Assert that experiments are deduplicated via map | ||||||||||||||||||||||||||||||||||||||||
| exps := config.GetExperiments() | ||||||||||||||||||||||||||||||||||||||||
| count := 0 | ||||||||||||||||||||||||||||||||||||||||
| for _, exp := range exps { | ||||||||||||||||||||||||||||||||||||||||
| if exp == experiment.Placeholder { | ||||||||||||||||||||||||||||||||||||||||
| count++ | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, 1, count, "experiment should appear exactly once") | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| t.Run("Loads valid experiments and enabled default experiments", func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -227,7 +247,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(`{"experiments":[]}`) // No experiments | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(`{"experiments":{}}`) // No experiments | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -244,7 +264,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| assert.True(t, config.WithExperimentOn(experiment.Placeholder)) | ||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, mockOutput.String(), "active project experiments: []") | ||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, mockOutput.String(), fmt.Sprintf("active permanently enabled experiments: [%s]", experiment.Placeholder)) | ||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, []experiment.Experiment{experiment.Placeholder}, config.experiments) | ||||||||||||||||||||||||||||||||||||||||
| assert.ElementsMatch(t, []experiment.Experiment{experiment.Placeholder}, config.GetExperiments()) | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| t.Run("Logs an invalid experiments in project configs", func(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -257,7 +277,7 @@ func Test_Config_WithExperimentOn(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(fmt.Sprintf("{\"experiments\":[\"%s\"]}", "experiment-37")) | ||||||||||||||||||||||||||||||||||||||||
| jsonContents := []byte(`{"experiments":{"experiment-37":true}}`) | ||||||||||||||||||||||||||||||||||||||||
| err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600) | ||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -283,7 +303,7 @@ func Test_Config_GetExperiments(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||
| _, mockPrintDebug := setupMockPrintDebug() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Write contents via config.json | ||||||||||||||||||||||||||||||||||||||||
| var configJSON = []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\",\"%s\"]}", validExperiment, validExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| var configJSON = []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, validExperiment)) | ||||||||||||||||||||||||||||||||||||||||
| _ = afero.WriteFile(fs, pathToConfigJSON, configJSON, 0600) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Set contexts of experiment flag | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Copyright 2022-2026 Salesforce, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package config | ||
|
|
||
| import "encoding/json" | ||
|
|
||
| // unmarshalExperimentsField handles backwards-compatible unmarshaling of the | ||
| // experiments field. It accepts both the old array format (["charm", "sandboxes"]) | ||
| // and the new map format ({"charm": true, "sandboxes": true}). | ||
| func unmarshalExperimentsField(raw json.RawMessage) map[string]bool { | ||
| if len(raw) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| // Try new map format first | ||
| var mapFormat map[string]bool | ||
| if err := json.Unmarshal(raw, &mapFormat); err == nil { | ||
| return mapFormat | ||
| } | ||
|
|
||
| // Fall back to old array format | ||
| var arrayFormat []string | ||
| if err := json.Unmarshal(raw, &arrayFormat); err == nil { | ||
| if len(arrayFormat) == 0 { | ||
| return nil | ||
| } | ||
| result := make(map[string]bool, len(arrayFormat)) | ||
| for _, exp := range arrayFormat { | ||
| result[exp] = true | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| return nil | ||
| } |
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.
🪬 question: Can we continue to use strict types of
experimentor is this not allowed as a key?