Skip to content
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.

Sign up for GitHub

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

fix: 🪶 experiment normalised-upload-paths was left out of the list of available experiments #2076

Merged
merged 3 commits into from
May 10, 2023
Merged

fix: 🪶 experiment normalised-upload-paths was left out of the list of available experiments #2076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b43c33b
fix: 🪶 experiment `normalised-upload-paths` was left out of the list …
MatthewDolan May 9, 2023
df0368e
fix: nit: update additional experiments to reference the experiments …
MatthewDolan May 9, 2023
bc027c5
fix: accidentally used american spelling on variable name which does …
MatthewDolan May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agent/artifact_uploader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (a *ArtifactUploader) Collect() (artifacts []*api.Artifact, err error) {
return nil, fmt.Errorf("resolving relative path for file %s: %w", file, err)
}

if experiments.IsEnabled("normalised-upload-paths") {
if experiments.IsEnabled(experiments.NormalisedUploadPaths) {
// Convert any Windows paths to Unix/URI form
path = filepath.ToSlash(path)
}
Expand Down
6 changes: 3 additions & 3 deletions agent/artifact_uploader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestCollect(t *testing.T) {
// path.Join function instead (which uses Unix/URI-style path separators,
// regardless of platform)

experimentKey := "normalised-upload-paths"
experimentKey := experiments.NormalisedUploadPaths
experimentPrev := experiments.IsEnabled(experimentKey)
defer func() {
if experimentPrev {
Expand All @@ -109,14 +109,14 @@ func TestCollect(t *testing.T) {
experiments.Disable(experimentKey)
}
}()
experiments.Disable("normalised-upload-paths")
experiments.Disable(experimentKey)
artifactsWithoutExperimentEnabled, err := uploader.Collect()
if err != nil {
t.Fatalf("[normalised-upload-paths disabled] uploader.Collect() error = %v", err)
}
assert.Equal(t, 5, len(artifactsWithoutExperimentEnabled))

experiments.Enable("normalised-upload-paths")
experiments.Enable(experimentKey)
artifactsWithExperimentEnabled, err := uploader.Collect()
if err != nil {
t.Fatalf("[normalised-upload-paths enabled] uploader.Collect() error = %v", err)
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ func (b *Bootstrap) defaultCheckoutPhase(ctx context.Context) error {
var mirrorDir string

// If we can, get a mirror of the git repository to use for reference later
if experiments.IsEnabled(`git-mirrors`) && b.Config.GitMirrorsPath != "" && b.Config.Repository != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha, i was foolish in searching only for "git-mirrors". nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, bad habit of mine is using `` for const-ish strings.

if experiments.IsEnabled(experiments.GitMirrors) && b.Config.GitMirrorsPath != "" && b.Config.Repository != "" {
b.shell.Commentf("Using git-mirrors experiment 🧪")
span.AddAttributes(map[string]string{"checkout.is_using_git_mirrors": "true"})
mirrorDir, err = b.getOrUpdateMirrorDir(ctx, b.Repository)
Expand Down Expand Up @@ -1442,7 +1442,7 @@ func (b *Bootstrap) defaultCheckoutPhase(ctx context.Context) error {
if err != nil {
b.shell.Warningf("Failed to enumerate git submodules: %v", err)
} else {
mirrorSubmodules := experiments.IsEnabled(`git-mirrors`) && b.Config.GitMirrorsPath != ""
mirrorSubmodules := experiments.IsEnabled(experiments.GitMirrors) && b.Config.GitMirrorsPath != ""
for _, repository := range submoduleRepos {
submoduleArgs := append([]string(nil), args...)
// submodules might need their fingerprints verified too
Expand Down
2 changes: 2 additions & 0 deletions experiments/experiments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const (
DescendingSpawnPrioity = "descending-spawn-priority"
InbuiltStatusPage = "inbuilt-status-page"
AgentAPI = "agent-api"
NormalisedUploadPaths = "normalised-upload-paths"
)

var (
Expand All @@ -27,6 +28,7 @@ var (
DescendingSpawnPrioity: {},
InbuiltStatusPage: {},
AgentAPI: {},
NormalisedUploadPaths: {},
}

experiments = make(map[string]bool, len(Available))
Expand Down