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.

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

Initial working commit #136

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Initial working commit #136

merged 2 commits into from
Jul 15, 2024

Conversation

chrisghill
Copy link
Contributor

Pretty simple change that touched a lot of files. Pulling out the afero mockfilesystem stuff. It was useful in testing, but some of our dependencies (airlock) perform filesystem access which makes it very difficult to maintain. There was really only 3 categories of changes here:

  • removing filesystem arguments from function signatures
  • changing afero filesystem calls (ReadFile, WriteFile, etc) to their corresponding functions in os or filepath
  • updating tests to perform their actions in a TempDir.

@chrisghill
Copy link
Contributor Author

Only remaining question is what to call the mockfilesystem package now that it isn't really a mock. The functions all remain, but they are intended to be used within a testing.TempDir.

I thought about testing, but that conflicts with the stdlib testing package. Perhaps testfilesystem, or testingutils but I don't like utils in packages.

Open to thoughts/suggestion.

Copy link
Contributor

@xpositivityx xpositivityx left a comment

Choose a reason for hiding this comment

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

This looks good!

Copy link
Contributor

@mclacore mclacore left a comment

Choose a reason for hiding this comment

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

Looks good! Approved. Left some suggestions though

@@ -107,8 +106,7 @@ func getDescription(t *templatecache.TemplateData) error {
var ignoredTemplateDirs = map[string]bool{"alpha": true}

func getTemplate(t *templatecache.TemplateData) error {
var fs = afero.NewOsFs()
cache, _ := templatecache.NewBundleTemplateCache(templatecache.GithubTemplatesFetcher, fs)
cache, _ := templatecache.NewBundleTemplateCache(templatecache.GithubTemplatesFetcher)
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a lint recommendation below, won't allow me to comment on it

@@ -39,7 +38,7 @@ func (b *Bundle) WriteSchemas(buildPath string, fs afero.Fs) error {

filepath := fmt.Sprintf("/schema-%s.json", task.label)

err = afero.WriteFile(fs, path.Join(buildPath, filepath), content, 0755)
err = os.WriteFile(path.Join(buildPath, filepath), content, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

would change perms to 0600 or add lint exclusion like //nolint:gosec to L17

writePath := "."
testDir := t.TempDir()
rootTemplateDir := path.Join(testDir, "/home/md-cloud")
writePath := testDir
Copy link
Contributor

Choose a reason for hiding this comment

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

would look nicer to remove L20 and replace all writePath to testDir, or replace testDir with writePath

@@ -302,7 +300,7 @@ func (h *Handler) postConnections(w http.ResponseWriter, r *http.Request) {
return
}

err = afero.WriteFile(h.fs, path.Join(h.bundleDir, "src", bundle.ConnsFile), bytes, 0755)
err = os.WriteFile(path.Join(h.bundleDir, "src", bundle.ConnsFile), bytes, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment about lint as above

@@ -83,7 +83,7 @@ func generateTfVarsFiles(buildPath, stepPath string, b *bundle.Bundle, fs afero.
}

filePath := fmt.Sprintf("/_%s_variables.tf.json", task.label)
err = afero.WriteFile(fs, path.Join(buildPath, stepPath, filePath), content, 0755)
err = os.WriteFile(path.Join(buildPath, stepPath, filePath), content, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

emptyConnections := checkEmptySchema(b.Connections)

if emptyConnections {
err := afero.WriteFile(fs, path, []byte("{}"), 0755)
err := os.WriteFile(path, []byte("{}"), 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

@@ -40,7 +40,7 @@ func transpileConnectionVarFile(path string, b *bundle.Bundle, fs afero.Fs) erro
return err
}

err = afero.WriteFile(fs, path, bytes, 0755)
err = os.WriteFile(path, bytes, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

emptyParams := checkEmptySchema(b.Params)

if emptyParams {
err := afero.WriteFile(fs, path, []byte("{}"), 0755)
err := os.WriteFile(path, []byte("{}"), 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@@ -59,7 +59,7 @@ func transpileAndWriteDevParams(path string, b *bundle.Bundle, fs afero.Fs) erro
return err
}

err = afero.WriteFile(fs, path, bytes, 0755)
err = os.WriteFile(path, bytes, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@chrisghill chrisghill merged commit b931184 into main Jul 15, 2024
3 of 4 checks passed
@chrisghill chrisghill deleted the remove-afero-mock-filesystem branch July 15, 2024 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants